Re: XSS and string interpolation

2012-06-28 Thread Alex Ogier
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.



Question about DecimalFields and admin widgets.

2012-06-28 Thread Serge G. Spaolonzi
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

2012-06-28 Thread Luke Plant
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.



Re: XSS and string interpolation

2012-06-28 Thread Alex Ogier
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

2012-06-28 Thread Alex Ogier
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

2012-06-28 Thread Jeremy Dunck
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: Customizable Serialization check-in

2012-06-28 Thread Piotr Grabowski

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.



XSS and string interpolation

2012-06-28 Thread Luke Plant
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: Newline stripping in templates: the dnl way

2012-06-28 Thread Stephen Kelly
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.