include tag security hole

4 views
Skip to first unread message

Czubakabra

unread,
Jul 21, 2007, 12:07:04 PM7/21/07
to Django developers
Hi,
Include tag is vulnerable to directory traversal:

{% include "/etc/passwd" %}

Django templates shoudn`t permit html coder to include files located
above TEMPLATE_DIRS paths.
What do you think about it?

Best regards,
Czubakabra

oggie rob

unread,
Jul 21, 2007, 2:37:18 PM7/21/07
to Django developers
Perhaps simply by preventing absolute paths? That would be very easy
to change if it doesn't prevent a legitimate setup.

Of course, html coders need to accept a certain responsibility because
sometimes they can access a *lot* of information quite easily. I would
think if you have a non programmer making changes, the programmers
would want to at least review those changes before accepting them, in
addition to a reasonable API.

-rob

Czubakabra

unread,
Jul 21, 2007, 6:13:05 PM7/21/07
to Django developers
Hello,

> Of course, html coders need to accept a certain responsibility because
> sometimes they can access a *lot* of information quite easily. I would
> think if you have a non programmer making changes, the programmers
> would want to at least review those changes before accepting them, in
> addition to a reasonable API.

I cannot agree with you. My application permit any user modify
templates code - follow a django philosophy - and they shouldn`t have
access to any private information that aren`t located in django base
path. Include tag is part of standard django library, and in my
opinion it contain trivial security hole... I made patch fixing this
vulnerability, but I want to see your opinions about it shoud be
patched or not.

Best regards,
Czubakabra

James Bennett

unread,
Jul 21, 2007, 6:29:16 PM7/21/07
to django-d...@googlegroups.com
On 7/21/07, Czubakabra <czuba...@o2.pl> wrote:
> Django templates shoudn`t permit html coder to include files located
> above TEMPLATE_DIRS paths.
> What do you think about it?

I'm personally ambivalent about where the "include" tag should be able
to search, because I can see cases where it'd be useful to have it
pull in things that aren't in TEMPLATE_DIRS. If you're interested in
confining the places from which the file may be pulled, use the "ssi"
tag and specify ALLOWED_INCLUDE_ROOTS in your settings file -- the
"ssi" tag will forbid access to directories not specified in that
setting.

Also, for future reference, Django's policy for reporting
security-related issues or suspected security-related issues is
outlined here:

http://www.djangoproject.com/documentation/contributing/#reporting-security-issues

And do keep in mind that the Django template system isn't necessarily
constructed to be safe when exposed to arbitrary users -- there are
many, many things which are perfectly acceptable and extremely useful
when the template author is a trusted member of your staff, but which
can easily be exploited if you allow arbitrary users to begin
authoring templates. I'm not certain we should weaken the template
language to accommodate that use case, since what you're doing
(allowing users to upload something which will be executed by your
server) is inherently insecure.

--
"Bureaucrat Conrad, you are technically correct -- the best kind of correct."

Gary Wilson

unread,
Jul 22, 2007, 2:21:36 AM7/22/07
to django-d...@googlegroups.com
Czubakabra wrote:
> Hi,
> Include tag is vulnerable to directory traversal:
>
> {% include "/etc/passwd" %}

It's a bug and not intended behavior. I've opened a ticket and have
attached a patch.

http://code.djangoproject.com/ticket/4952

Gary

SmileyChris

unread,
Jul 22, 2007, 7:59:57 PM7/22/07
to Django developers
On Jul 22, 6:21 pm, Gary Wilson <gary.wil...@gmail.com> wrote:
> It's a bug and not intended behavior. I've opened a ticket and have
> attached a patch.
>
> http://code.djangoproject.com/ticket/4952

I've put up a new patch which is pretty solid and ready for a
committer's review.

Tai Lee

unread,
Jul 22, 2007, 8:37:40 PM7/22/07
to Django developers
On Jul 22, 8:29 am, "James Bennett" <ubernost...@gmail.com> wrote:

> I'm personally ambivalent about where the "include" tag should be able
> to search, because I can see cases where it'd be useful to have it
> pull in things that aren't in TEMPLATE_DIRS. If you're interested in
> confining the places from which the file may be pulled, use the "ssi"
> tag and specify ALLOWED_INCLUDE_ROOTS in your settings file -- the
> "ssi" tag will forbid access to directories not specified in that
> setting.

Don't you mean *disallow* use of {% include %} and *enforce* use of {%
ssi %} (only)? If he has no control over the template code html
authors are generating, and {% include %} is part of django, those
template authors would be able to access it and would expect to be
allowed to access it?

James Bennett

unread,
Jul 22, 2007, 8:50:02 PM7/22/07
to django-d...@googlegroups.com
On 7/22/07, Tai Lee <mrmac...@gmail.com> wrote:
> Don't you mean *disallow* use of {% include %} and *enforce* use of {%
> ssi %} (only)? If he has no control over the template code html
> authors are generating, and {% include %} is part of django, those
> template authors would be able to access it and would expect to be
> allowed to access it?

Sort of, but not really. Personally, I don't think there's any secure
way to allow arbitrary users to upload templates, because there are
just too many ways to expose something you wouldn't want to expose; if
you were going to develop a sandboxed version of the template system,
you'd have to strip it down so much that it almost wouldn't be
recognizable afterward.

oggie rob

unread,
Jul 22, 2007, 11:23:33 PM7/22/07
to Django developers
> Sort of, but not really. Personally, I don't think there's any secure
> way to allow arbitrary users to upload templates, because there are
> just too many ways to expose something you wouldn't want to expose; if
> you were going to develop a sandboxed version of the template system,
> you'd have to strip it down so much that it almost wouldn't be
> recognizable afterward.

This was my feeling also, however I think it is prudent to prevent
legitimate users from making mistakes. I don't know about designers,
but I have worked with some programmers that just don't think things
through like they should...
You mentioned earlier that you "can see cases where it'd be useful to
have it pull in things that aren't in TEMPLATE_DIRS" - I suppose I can
also, but everything I can think of is safer & cleaner by restricting
it to TEMPLATE_DIRS locations (e.g. adding the extra directories, sym
linking, or just maintaining another copy in an existing TEMPLATE_DIRS
spot). The advantage is of course that the decision is made by the
person managing the settings.py file - which is usually the one who
knows more about system security than developers. This isn't
foolproof, but does stop some mistakes, and for that reason I think
the fix should be included.

-rob

Tom Tobin

unread,
Jul 22, 2007, 11:29:03 PM7/22/07
to django-d...@googlegroups.com
On 7/22/07, oggie rob <oz.rob...@gmail.com> wrote:
>
> This was my feeling also, however I think it is prudent to prevent
> legitimate users from making mistakes. I don't know about designers,
> but I have worked with some programmers that just don't think things
> through like they should...

This tells me they have a much deeper problem that Django really can't
help solve. ;-)

James Bennett

unread,
Jul 22, 2007, 11:33:20 PM7/22/07
to django-d...@googlegroups.com
On 7/22/07, oggie rob <oz.rob...@gmail.com> wrote:
> but everything I can think of is safer & cleaner by restricting
> it to TEMPLATE_DIRS locations (e.g. adding the extra directories, sym
> linking, or just maintaining another copy in an existing TEMPLATE_DIRS
> spot). The advantage is of course that the decision is made by the
> person managing the settings.py file - which is usually the one who
> knows more about system security than developers. This isn't
> foolproof, but does stop some mistakes, and for that reason I think
> the fix should be included.

OK, then I'd agree on restricting it to TEMPLATE_DIRS.

I'm just worried that this would create the misconception that it's
safe to allow arbitrary third parties to supply templates; I'd be
willing to bet money that any site doing this is open to very serious
exploits: XSS becomes trivially easy, user session information can be
obtained, etc., etc.

Again, to lock things down enough to be safe for arbitrary user
templates, you'd basically have to make the template system
unrecognizable.

oggie rob

unread,
Jul 22, 2007, 11:45:14 PM7/22/07
to Django developers
> > This was my feeling also, however I think it is prudent to prevent
> > legitimate users from making mistakes. I don't know about designers,
> > but I have worked with some programmers that just don't think things
> > through like they should...
>
> This tells me they have a much deeper problem that Django really can't
> help solve. ;-)

I can always hope for a nice contrib app - one that automatically
fires people for poorly written code would probably do the trick!

Gary Wilson

unread,
Jul 22, 2007, 11:53:06 PM7/22/07
to django-d...@googlegroups.com

I think the patch looks good. Can someone please confirm that the
latest patch works ok on Windows.

Gary

SmileyChris

unread,
Jul 23, 2007, 12:11:18 AM7/23/07
to Django developers
On Jul 23, 3:53 pm, Gary Wilson <gary.wil...@gmail.com> wrote:

> SmileyChris wrote:
> I think the patch looks good. Can someone please confirm that the
> latest patch works ok on Windows.

I guess you mean apart from me? ;)

Gary Wilson

unread,
Jul 23, 2007, 12:19:51 AM7/23/07
to django-d...@googlegroups.com

You would be fine, but I wasn't sure if you tested on Windows.

Gary

SmileyChris

unread,
Jul 23, 2007, 12:50:05 AM7/23/07
to Django developers
On Jul 23, 4:19 pm, Gary Wilson <gary.wil...@gmail.com> wrote:
> You would be fine, but I wasn't sure if you tested on Windows.
Yea, my development box runs XP.
PS: I can't patch your diffs because they don't use the format which
TortoiseSVN accepts and the win32 build of patch falls over on it too.
How are you making them?


Gary Wilson

unread,
Jul 23, 2007, 1:21:36 AM7/23/07
to django-d...@googlegroups.com
SmileyChris wrote:
> PS: I can't patch your diffs because they don't use the format which
> TortoiseSVN accepts and the win32 build of patch falls over on it too.
> How are you making them?

I'm making my diffs with Bazaar, using "bzr diff". My unix patch seems
to handle them ok. Sorry about that.

Gary

Reply all
Reply to author
Forward
0 new messages