Agreed that this is an XSS security issue -  Thank you for demonstrating it
!

I wrongly assumed that json.dumps() will HTML-escape the strings passed to
it - apparently it lacks the option to do so, even if it's in the JSON spec.

I have a modified patch that:
- provides Python-to-JSON dump in a template filter
- said template filter escapes HTML content, keeping the output
JSON-compatible
- modifies all occurences of json.dumps in views.py to use this filter

In the same patch I have several other security fixes, since the XSS path
remained open inside Angular, or for the commands sent to the server.

Can you please review the patch at:

http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=toaster/master

Cheers,
Alex




On Tue, Nov 11, 2014 at 11:31 AM, Michael Wood <[email protected]>
wrote:

> Hi,
>
> Valid json can contain unescaped markup tags which will break the
> javascript e.g. if you put your project name to "</script><h2>Hi mum<!-- or
> worse some javascript --></h2>" the project page will interpret that.
>
> http://jsfiddle.net/uLpecL5o/
>
> The escapejs filter will escape all the correct characters the resulting
> string of the json can then be safely parsed by the browser.
> If we want to use |safe we really need to be sure that data is safe, which
> may mean that instead we sanitise it before storing it.
>
> Oops yes too long working with jinja2 which is based on django got
> confused there!
>
> Michael
>
> On 07/11/14 16:52, Damian, Alexandru wrote:
>
>> Hi,
>>
>> Hi, this is a good point you raise here - there are some aspects that
>> need considering, though -
>>
>>     the data coming in this page (e.g. prj, builds, etc..) is already
>> coming as JSON, the conversion is done in the view. Here we mark the value
>> as not needing any further escape (through the safe filter) because we know
>> it's already a valid json string.
>>
>> json is already valid javascript code, so we don't need to parse it
>> manually, the browser will interpret it as such.
>>
>> btw, we're not using jinja2 templating engine, we use the built-in django
>> templating engine :)
>>
>>
>> Cheers,
>> Alex
>>
>>
>>
>> On Thu, Nov 6, 2014 at 4:11 PM, Michael Wood <[email protected]
>> <mailto:[email protected]>> wrote:
>>
>>     When passing the data from the jinja2 template to javascript make sure
>>     we escape and parse the JSON to avoid any invalid values being
>>     interpreted.
>>
>>     Signed-off-by: Michael Wood <[email protected]
>>     <mailto:[email protected]>>
>>
>>     ---
>>      bitbake/lib/toaster/toastergui/templates/project.html | 14
>>     +++++++-------
>>      1 file changed, 7 insertions(+), 7 deletions(-)
>>
>>     diff --git a/bitbake/lib/toaster/toastergui/templates/project.html
>>     b/bitbake/lib/toaster/toastergui/templates/project.html
>>     index 6a81283..00fb2b4 100644
>>     --- a/bitbake/lib/toaster/toastergui/templates/project.html
>>     +++ b/bitbake/lib/toaster/toastergui/templates/project.html
>>     @@ -335,13 +335,13 @@ angular.element(document).ready(function() {
>>        scope.urls.layers = "{% url 'layers' %}";
>>        scope.urls.targets = "{% url 'targets' %}";
>>        scope.urls.importlayer = "{% url 'importlayer'%}"
>>     -  scope.project = {{prj|safe}};
>>     -  scope.builds = {{builds|safe}};
>>     -  scope.layers = {{layers|safe}};
>>     -  scope.targets = {{targets|safe}};
>>     -  scope.frequenttargets = {{freqtargets|safe}};
>>     -  scope.machine = {{machine|safe}};
>>     -  scope.releases = {{releases|safe}};
>>     +  scope.project = JSON.parse ("{{prj|escapejs}}");
>>     +  scope.builds = JSON.parse ("{{builds|escapejs}}");
>>     +  scope.layers = JSON.parse ("{{layers|escapejs}}");
>>     +  scope.targets = JSON.parse ("{{targets|escapejs}}");
>>     +  scope.frequenttargets = JSON.parse ("{{freqtargets|escapejs}}");
>>     +  scope.machine = JSON.parse ("{{machine|escapejs}}");
>>     +  scope.releases = JSON.parse ("{{releases|escapejs}}");
>>
>>        scope.zone1alerts = [];
>>        scope.zone2alerts = [];
>>     --
>>     1.9.1
>>
>>     --
>>     _______________________________________________
>>     toaster mailing list
>>     [email protected] <mailto:[email protected]>
>>     https://lists.yoctoproject.org/listinfo/toaster
>>
>>
>>
>>
>> --
>> Alex Damian
>> Yocto Project
>> SSG / OTC
>>
>> ---------------------------------------------------------------------
>> Intel Corporation (UK) Limited
>> Registered No. 1134945 (England)
>> Registered Office: Pipers Way, Swindon SN3 1RJ
>> VAT No: 860 2173 47
>>
>> This e-mail and any attachments may contain confidential material for
>> the sole use of the intended recipient(s). Any review or distribution
>> by others is strictly prohibited. If you are not the intended
>> recipient, please contact the sender and delete all copies.
>>
>>
> ---------------------------------------------------------------------
> Intel Corporation (UK) Limited
> Registered No. 1134945 (England)
> Registered Office: Pipers Way, Swindon SN3 1RJ
> VAT No: 860 2173 47
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>



-- 
Alex Damian
Yocto Project
SSG / OTC
-- 
_______________________________________________
toaster mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/toaster

Reply via email to