Re: Newline stripping in templates: the dnl way
Leon Matthews wrote: > On 2 March 2012 09:45, Carl Meyer > wrote: >> Same reason any ticket stalls - it seems that nobody felt strongly >> enough about it to put the time into reviewing and thoroughly testing >> the patch and marking it Ready for Checkin. If you'd like to see it in >> (post 1.4 at this point, of course), feel free to do that! > > Done! :-) > > I've created a new version of the patch against Django 1.5-dev, which > passes all tests, etc... > > I've attached it to the ticket directly: > https://code.djangoproject.com/ticket/2594 > > Cheers, > > Leon > Oh, interesting that this is getting interest again. I think I emailed the list annually about it without getting a reply :). I'll review again soon too. Thanks, Steve. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
XSS and string interpolation
Hi all,
Django's code base has quite a few instances of string interpolation
being used to build up HTML e.g.:
contrib/admin/util.py
return mark_safe('%s: %s' %
(escape(capfirst(opts.verbose_name)),
admin_url,
escape(obj)))
The problem with this is that it is easy to forget to use 'escape',
giving rise to an XSS vulnerability. This has led to at least one
instance in the past:
https://github.com/django/django/commit/9f6d50d02e
(I couldn't find others, there might be. I'm sure there could be many in
3rd party code - there was at least one instance in some code I wrote).
It also makes review hard. In the above fragment, is it still vulnerable
to XSS due to the unescaped 'admin_url'? (AFAICS after looking at it,
'no', but it's not that easy to tell).
I'm going to clean these up using the following pattern:
return html_fragment('%s: %s',
capfirst(opts.verbose_name),
admin_url,
obj)
The implementation, which will be added to django.utils.html, is just:
def html_fragment(template, *args):
return mark_safe(template % tuple(map(conditional_escape, args)))
This is an all round win:
* It is secure by default, like autoescaping in templates
* It is easier to use - you only have to import 'html_fragment', instead
of both 'mark_safe' and 'escape'
* The code is easier to read due to fewer parenthesis
* It makes XSS problems more greppable. You now have to grep for:
* all string interpolations using % (which should be removed and
replaced with html_fragment if the result is eventually
mark_safe'd).
As before, you have to audit:
* mark_safe (but this will have far fewer instances now)
* |safe
* {% autoescape off %}
This utility will be public and documented, and suggested as an
alternative to ever using 'mark_safe' or 'escape' directly.
Apart from letting people know, I've got a few reasons for emailing
about this:
1) Are there are any better alternatives? I'm ruling out use of
'Template' because it is overly verbose for this use case, both in
API and template syntax.
2) Any better name than 'html_fragment'?
3) It made me realise we should have thought of this earlier, and
that our security procedures need improving in this regard.
When we had the XSS exploit I mentioned above, we simply fixed
it by applying 'escape'. We didn't ask "why did this happen?
What is the root cause? How can we stop it ever happening again?"
We need to be much more rigorous about applying things like
the "5 Whys" http://en.wikipedia.org/wiki/5_Whys
especially for security problems.
Regards,
Luke
--
Parenthetical remarks (however relevant) are unnecessary
Luke Plant || http://lukeplant.me.uk/
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
Re: Customizable Serialization check-in
W dniu 26.06.2012 11:52, Tom Christie pisze:
> It is the way I am doing deserialization - pass instance to subfields
Seems fine. It's worth keeping in mind that there's two ways around
of doing this.
1. Create an empty instance first, then populate it with the field
values in turn.
2. Populate a dictionary with the field values first, and then create
an instance using those values.
The current deserialization does something closer to the second.
I don't know if there's any issues with doing things the other way
around, but you'll want to consider which makes more sense.
Second approach assume that every field returns some value. But what if
we don't want to deserialize some field? In my deserialization instance
is passed to field and field will eventually fill it with some value.
def deserialize_value(self, obj, instance, field_name):
setattr(instance, field_name, obj)
If we don't want to deserialize field we simply do nothing in
deserialize_value.
If second approach is used we must return value. Some idea is to mark
field as not deserializable:
class MyField(Field):
deserializable = False
> Where I returned (native, attributes) I will return (native,
metainfo). It's only matter of renaming but metainfo will be more than
attributes.
Again, there's two main ways around I can think of for populating
metadata such as xml attributes.
1. Return the metadata upfront to the renderer.
2. Include some way for the renderer to get whatever metadata it needs
at the point it's needed.
This is one point where what I'm doing in django-serializers differs
from your work, in that rather than return extra metadata upfront, the
serializers return a dictionary-like object (that e.g. can be directly
serialized to json or yaml), that also includes a way of returning the
fields for each key (so that e.g. the xml renderer can call
field.attributes() when it's rendering each field.)
Again, you might decide that (1) makes more sense, but it's worth
considering.
As ever, if there's any of this you'd like to talk over off-list, feel
free to drop me a mail - t...@tomchristie.com
Regards,
Tom
I rewrite this so it's more similar to django-serializers.
But from the beginning - what I do in this week? :)
I agreed that xml attributes in my solution are overstated. So I want
to modify it. Attributes in xml are one of (two) ways of presenting
information. I still want to have field for attributes, but doing it in
this way:
class MyField(Field):
attr1 = Field()
attr2 = Field()
def serialized_value(self, obj, field_name):
return field_value
def metainfo(self):
return {'attributes' : ['attr1', 'attr2']}
JSON will skip attributes at all:
some_field : field_value
XML will render it:
field_value
If metainfo won't return dict with attributes XML will render this:
val1
val2
field_value
I code it like django-serializers's DictWithMeta but I added one more
functionality to represent Field that have subfields and one extra
value. I'm still not convicted it is good solution, so I rewrite it
several times but always end up with something like that :)
I will push code tomorrow because I still want to do some tweaks.
--
Piotr Grabowski
--
You received this message because you are subscribed to the Google Groups "Django
developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
Re: XSS and string interpolation
On Jun 28, 2012, at 6:57 AM, Luke Plant wrote: > Hi all, > > 2) Any better name than 'html_fragment'? > I like the general approach, but I miss the security-minded namse of "escape" and "mark safe". Maybe "safe_html_fragment" or "make_safe_html_fragment"? Getting annoyingly long, I know. (Apologies if this feels bike shedding) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: XSS and string interpolation
On Thu, Jun 28, 2012 at 10:52 AM, Jeremy Dunck wrote: > > On Jun 28, 2012, at 6:57 AM, Luke Plant wrote: > > > Hi all, > > > > 2) Any better name than 'html_fragment'? > > > > I like the general approach, but I miss the security-minded namse of > "escape" and "mark safe". Maybe "safe_html_fragment" or > "make_safe_html_fragment"? Getting annoyingly long, I know. > > (Apologies if this feels bike shedding) > It's not just bikeshedding. The "safe" word is a good indicator that the mark_safe utility function has been used throughout. In fact, given the tight relationship of the two, I might even suggest a name like "html_mark_safe," which strongly suggests "mark_safe with an html format string." Also, to be compatible with python 3 and more idiomatic python, I would implement the function as: def html_mark_safe(format_string, *args): return mark_safe(format_string.format(*map(conditional_escape, args))) Best, Alex Ogier -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: XSS and string interpolation
On Thu, Jun 28, 2012 at 11:18 AM, Alex Ogier wrote: > > Also, to be compatible with python 3 and more idiomatic python, I > would implement the function as: > > def html_mark_safe(format_string, *args): > return mark_safe(format_string.format(*map(conditional_escape, args))) > Actually, on second thought, this is even better: def html_mark_safe(format_string, *args, **kwargs): args_safe = map(conditional_escape, args) kwargs_safe = dict([(k, conditional_escape(v)) for (k, v) in kwargs.iteritems()]) return mark_safe(format_string.format(*args_safe, **kwargs_safe)) That's an HTML-safe replacement of the str.format() method, so far as I can tell (except that all parameters must be [safe-]strings). That allows more idiomatic python, and won't require awkward shims in python 3, but it would mean that you can't directly translate % interpolations. I think thats a good tradeoff. Best, Alex Ogier -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: XSS and string interpolation
On 28/06/12 16:32, Alex Ogier wrote:
> That's an HTML-safe replacement of the str.format() method, so far as
> I can tell (except that all parameters must be [safe-]strings). That
> allows more idiomatic python, and won't require awkward shims in
> python 3, but it would mean that you can't directly translate %
> interpolations. I think thats a good tradeoff.
Yeah, I think it makes sense to move to str.format at this point. It
also helps ensure that, if you are switching code, you don't
accidentally pass a string that is already %-interpolated:
html_fragment("Some stuff %s" % data)
As for the name, I'm not convinced by html_mark_safe - to my mind that
implies an HTML version of 'mark_safe', which doesn't make sense - you
should only be using 'mark_safe' on html anyway.
Jeremy wrote:
> I like the general approach, but I miss the security-minded namse of
> "escape" and "mark safe". Maybe "safe_html_fragment" or
> "make_safe_html_fragment"? Getting annoyingly long, I know.
>
> (Apologies if this feels bike shedding)
No problem with bike-shedding - that's why I asked the question. My
response would be that the name 'Template' also doesn't imply anything
about security, but function. The same is true here - 'html_fragment' is
for building HTML fragments. Of course it is secure! Why would we call
it that if it wasn't fit for purpose? :-)
Actually my main objection is that it's a bit long, the above is just
rationalisation.
Some other alternatives: build_html, build_html_safe, format_html
Luke
--
Parenthetical remarks (however relevant) are unnecessary
Luke Plant || http://lukeplant.me.uk/
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
Question about DecimalFields and admin widgets.
Hi,
I wonder why does't the DecimalFields implement an Admin Widget like the
other Fields?.
I am working in a application based in the Admin site and I am in trouble
trying to add some style to the decimal fields because they are rendered
like a plain input text field with no class associated. For the other
fields I have no problem because the have a class associated based in the
widget the implement.
django/contrib/admin/options.py
-
models.DateField: {'widget': widgets.AdminDateWidget},
models.TimeField: {'widget': widgets.AdminTimeWidget},
models.TextField: {'widget': widgets.AdminTextareaWidget},
models.URLField:{'widget': widgets.AdminURLFieldWidget},
models.IntegerField:{'widget': widgets.AdminIntegerFieldWidget},
models.BigIntegerField: {'widget': widgets.AdminBigIntegerFieldWidget},
models.CharField: {'widget': widgets.AdminTextInputWidget},
models.ImageField: {'widget': widgets.AdminFileWidget},
models.FileField: {'widget': widgets.AdminFileWidget},
-
Thanks
Serge
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
Re: XSS and string interpolation
On Thu, Jun 28, 2012 at 1:14 PM, Luke Plant wrote: > > Some other alternatives: build_html, build_html_safe, format_html > +1 for format_html. Best, Alex Ogier -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
