[OSSA-2014-010] XSS in Horizon-Orchestration (CVE-2014-0157)

[Cristian Fiorentino]

*Description*
XSS vulnerability identified in Horizon-Orchestration while uploading a stack template.
Arbitrary Javascript code may be introduced via the "Description" fields of Heat templates; such code was found to be executed by Horizon.

*Threat Description*
-Potential Adversaries: malicious Heat templates owners/malicious Heat templates catalogs.
-Potential Assets: horizon user/admin access credentials (session cookies/CSRF tokens), VMs/Network configuration/management, tenants confidential informartion, etc.
-Potential Threats: Malicious Heat template owner/catalog makes an Horizon user to utilize a malicious template, which once introduced in Horizon obtains user access credentials and send them back to the attacker.

*Environment*
One node with Devstack over Ubuntu13.10, latest Icehouse code, Firefox web browser and the following OpenStack configuration:
shell, key, horizon, g-reg, g-api, n-api, n-cpu, n-cond, n-crt, n-net, n-sch, n-novnc, n-xvnc, n-cauth, n-obj, c-api, c-sch, c-vol, ceilometer-acompute, ceilometer-acentral, ceilometer-collector, ceilometer-api, ceilometer-alarm-notifier, ceilometer-alarm-evaluator, h-eng, h-api, h-api-cfn, h-api-cw

*Steps to reproduce*
1- Sign-in to Horizon and click on Orchestration/Stack section.
2- Click on "Launch Stack"
3- Select "Direct input", and copy-paste into "Template data" the contents of some template (I have used:
https://github.com/openstack/heat-templates/blob/master/cfn/F17/AutoScalingMultiAZSample.template)
4- Update the contents of the DBUsername "Description" field with the following:
   "DBUsername": {
 ...
      "Description" : "<script>alert('XSS!!!')</script>",
 ...
    },
5- Click on Next
6- Being on the Launch Stack form, click on DBUsername text box as if you were going to modify its value.
7- A pop-up saying "XSS!!!" will appear, confirming the XSS vulnerability.

*How to fix*
- Perform input validation for "Description" fields in templates (need to take into account all template input methods: upload from URL, upload from file, direct input).
- Perform output sanitization when displaying template's "Description" messages.

[Kieran Spear (kspear)]

Confirmed in Havana using "Upload from file" and the attached template.

[Cristian Fiorentino (cristian-fiorentino)]

Having analyzed the issue, the problems seems to be that the template structure used to display the "Description" message is not performing autoescaping. I will be proving a patch proposal soon.

Question on the process: are Gerrit reviews on security bugs automatically being set to private, or should I perform any additional steps to maintain the review private? Thanks.

[Tristan Cacqueray (tristan-cacqueray)]

Hello Crisitan,

Our gerrit does not support private review, in order to get the patch private, you have to attach them to LP and get the review done here too.
Thank you in advance!

[Tristan Cacqueray (tristan-cacqueray)]

I'm really sorry, I meant cristian* !

Here is impact description #1

Title: XSS in Horizon-Orchestration
Reporter: Cristian Fiorentino
Products: Horizon
Versions: 2013.2.1 version up to 2013.2.2

Description:
Cristian Fiorentino from Intel reported a vulnerability in Horizon Orchestration dashboard. By tricking a Horizon user, a malicious templates owner/catalog, may trigger an XSS when a malicious template is used in the Orchestration/Stack section of Horizon, resulting in potential assets theft (Horizon user/admin access credentials (session cookies/CSRF tokens), VMs/Network configuration/management, tenants confidential informartion, etc.). Only setups using Heat and Horizon are affected.

[Cristian Fiorentino (cristian-fiorentino)]

Proposing a patch for this bug in the attachment.

[Cristian Fiorentino (cristian-fiorentino)]

Keep waiting for any feedback on the attached patch.
Details on the proposed fix are included in the patch itself.
Thanks.

[David Lyle (david-lyle)]

I have tried the patch and it works well to solve the issue. It's a nice general fix to catch this particular exploit in other forms as well.

[Cristian Fiorentino (cristian-fiorentino)]

Thanks much for the review David.
Please let me know if I am wrong, but I think there is still the need for further reviews on the proposed patch.

[Thierry Carrez (ttx)]

Impact description review:

* I would title it "XSS in Horizon orchestration dashboard"
* the "By tricking" sentence is confusing. i would say "By tricking a Horizon user into using a malicious template in the Orchestration/Stack section of Horizon, a remote attacker may trigger a cross-site-scripting vulnerability. It may result in..."
* the double parenthesis listing the effects is very confusing. I would simplify it a lot.
* "Only setups using Heat and Horizon are affected." -- do you need to use Heat, or just the orchestration dashboard ? Or would rendering a template in Horizon (without using Heat) also trigger the issue ? In doubt I would just say "Only setups exposing the orchestration dashboard in Horizon are affected".

[Tristan Cacqueray (tristan-cacqueray)]

@ttx Thanks! Concerning the affected setups, the orchestration dashboard should only be displayed if a heat service is defined. Which is better covered by your sentence anyway.

Here is impact description #2

Title: XSS in Horizon orchestration dashboard
Reporter: Cristian Fiorentino
Products: Horizon
Versions: 2013.2.1 version up to 2013.2.2

Description:
Cristian Fiorentino from Intel reported a vulnerability in Horizon Orchestration dashboard. By tricking a Horizon user into using a malicious template in the Orchestration/Stack section of Horizon, a remote attacker may trigger a cross-site-scripting vulnerability. It may result in potential assets theft (Horizon user/admin access credentials, tenants confidential information, etc.). Only setups exposing the orchestration dashboard in Horizon are affected.

[Thierry Carrez (ttx)]

Impact desc #2 looks good

[Kieran Spear (kspear)]

Thanks Cristian. The patch fixes the issue for me too. I would really like to understand *why* this help_text variable isn't autoescaped in the original template though. I haven't been able to answer that question.

BUT in reviewing this I've discovered another related issue that we'll need to fix at the same time:

openstack_dashboard/dashboards/project/stacks/templates/stacks/_detail_overview.html uses {% autoescape off %}

This makes Horizon vulnerable to script data inside the "outputs" section of a Heat template:

outputs:
  instance_ip:
    description: The IP address of the deployed instance
    value: "<script>alert('XSS!!!')</script>"

After a stack is created successfully, clicking through to the detail of that particular stack will run the script.

[Kieran Spear (kspear)]

The original issue is more complicated than it seems. Here's the HTML that's generated without the fix:

<span class="help-block"> &lt;script&gt;alert(&#39;XSS!!!&#39;)&lt;/script&gt; </span>

And with the fix:

<span class="help-block"> &amp;lt;script&amp;gt;alert(&amp;#39;XSS!!!&amp;#39;)&amp;lt;/script&amp;gt; </span>

So Django *is* autoescaping the help text. The patch above escapes the text a second time. This is also why when I tried making the patch neater by using {{ field.help_text|force_escape }}, it didn't fix the bug, as that doesn't result in double-escaping.

The real cause of the bug is feeding the escaped HTML from the <span> tag into bootstrap-tooltip:

  $(document).tooltip({
...snip...
    title: function () {
      return $(this).closest('div.form-field').children('.help-block').text();
    }
  });

Which inserts it straight back into the DOM with .html:

$tip.find('.tooltip-inner').html(this.getTitle())

So the text is effectively unescaped once the tooltip is triggered.

[Cristian Fiorentino (cristian-fiorentino)]

I could reproduce the second related issue identified by Kieran.
I will be providing a new patch including fixes for both issues.

[Cristian Fiorentino (cristian-fiorentino)]

Attaching a new patch which fixes the original and the later identified XSS cases in Horizon/Orchestration.

[Kieran Spear (kspear)]

I made a few changes to Cristian's patch. Fixed tests and modified the stack_output templatetag to work with escaping. Also added a comment to the template about the fix to the original bug.

[Thierry Carrez (ttx)]

horizon-core: please review propsoed patch and post your approval here as a comment

[Julie Pichon (jpichon)]

Thank you for looking into this, Cristian and Kieran. The patch in comment 16 looks good to me. I tested it on master and can confirm it works well. I haven't had a chance to test it on the Havana branch yet.

[Julie Pichon (jpichon)]

I encountered a minor conflict (on the imports) when applying the patch to the stable/havana branch, I'm attaching the Havana version. I can confirm this fixes both issues in a Havana devstack, thanks.

[David Lyle (david-lyle)]

The patch in #16 looks good to me. It fixes the issue.

[Tristan Cacqueray (tristan-cacqueray)]

There are some typo in the impact desc draft, Intel wasn't mentioned on the reporter line, and 2013.2 is also vulnerable, affected versions line is also updated:

Here is impact description #3:

Title: XSS in Horizon orchestration dashboard
Reporter: Cristian Fiorentino (Intel)
Products: Horizon
Versions: 2013.2 version up to 2013.2.3

Description:
Cristian Fiorentino from Intel reported a vulnerability in Horizon
Orchestration dashboard. By tricking a Horizon user into using a
malicious template in the Orchestration/Stack section of Horizon, a
remote attacker may trigger a cross-site-scripting vulnerability. It may
result in potential assets theft (Horizon user/admin access credentials,
tenants confidential information, etc.). Only setups exposing the
orchestration dashboard in Horizon are affected.

[Tristan Cacqueray (tristan-cacqueray)]

the pre-OSSA have been sent.

Proposed public disclosure date/time:
2014-04-08 15:00 UTC

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/havana
Review: https://review.openstack.org/86056

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/86059
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=198dba6a2d92b8157b86285f5b0cb000e64ae7ac
Submitter: Jenkins
Branch: master

commit 198dba6a2d92b8157b86285f5b0cb000e64ae7ac
Author: CristianFiorentino <email address hidden>
Date: Mon Mar 10 17:36:31 2014 -0300

    Introduces escaping in Horizon/Orchestration

    1) Escape help_text a second time to avoid bootstrap tooltip XSS issue

    The "Description" parameter in a Heat template is used to populate
    a help_text tooltip in the dynamically generated Heat form. Bootstrap
    inserts this tooltip into the DOM using .html() which undoes any
    escaping we do in Django (it should be using .text()).

    This was fixed by forcing the help_text content to be escaped a second
    time. The issue itself is mitigated in bootstrap.js release 2.0.3
    (ours is currently 2.0.1).

    2) Properly escape untrusted Heat template 'outputs'

    The 'outputs' parameter in a Heat template was included in a Django
    template with HTML autoescaping turned off. Malicious HTML content
    could be included in a Heat template and would be rendered by Horizon
    when details about a created stack were displayed.

    This was fixed by not disabling autoescaping and explicitly escaping
    untrusted values in any strings that are later marked "safe" to render
    without further escaping.

    Change-Id: Icd9f9d9ca77068b12227d77469773a325c840001
    Closes-Bug: #1289033
    Co-Authored-By: Kieran Spear <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/86054
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=1b0106e2804a45e641433c4bd459e6bed85521c3
Submitter: Jenkins
Branch: milestone-proposed

commit 1b0106e2804a45e641433c4bd459e6bed85521c3
Author: CristianFiorentino <email address hidden>
Date: Mon Mar 10 17:36:31 2014 -0300

    Introduces escaping in Horizon/Orchestration

    1) Escape help_text a second time to avoid bootstrap tooltip XSS issue

    The "Description" parameter in a Heat template is used to populate
    a help_text tooltip in the dynamically generated Heat form. Bootstrap
    inserts this tooltip into the DOM using .html() which undoes any
    escaping we do in Django (it should be using .text()).

    This was fixed by forcing the help_text content to be escaped a second
    time. The issue itself is mitigated in bootstrap.js release 2.0.3
    (ours is currently 2.0.1).

    2) Properly escape untrusted Heat template 'outputs'

    The 'outputs' parameter in a Heat template was included in a Django
    template with HTML autoescaping turned off. Malicious HTML content
    could be included in a Heat template and would be rendered by Horizon
    when details about a created stack were displayed.

    This was fixed by not disabling autoescaping and explicitly escaping
    untrusted values in any strings that are later marked "safe" to render
    without further escaping.

    Change-Id: Icd9f9d9ca77068b12227d77469773a325c840001
    Closes-Bug: #1289033
    Co-Authored-By: Kieran Spear <email address hidden>

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/86056
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=54ec015f720a4379e8ffc34345b3a7bf36b6f15b
Submitter: Jenkins
Branch: stable/havana

commit 54ec015f720a4379e8ffc34345b3a7bf36b6f15b
Author: CristianFiorentino <email address hidden>
Date: Mon Mar 10 17:36:31 2014 -0300

    Introduces escaping in Horizon/Orchestration

    1) Escape help_text a second time to avoid bootstrap tooltip XSS issue

    The "Description" parameter in a Heat template is used to populate
    a help_text tooltip in the dynamically generated Heat form. Bootstrap
    inserts this tooltip into the DOM using .html() which undoes any
    escaping we do in Django (it should be using .text()).

    This was fixed by forcing the help_text content to be escaped a second
    time. The issue itself is mitigated in bootstrap.js release 2.0.3
    (ours is currently 2.0.1).

    2) Properly escape untrusted Heat template 'outputs'

    The 'outputs' parameter in a Heat template was included in a Django
    template with HTML autoescaping turned off. Malicious HTML content
    could be included in a Heat template and would be rendered by Horizon
    when details about a created stack were displayed.

    This was fixed by not disabling autoescaping and explicitly escaping
    untrusted values in any strings that are later marked "safe" to render
    without further escaping.

    Conflicts:
     openstack_dashboard/dashboards/project/stacks/mappings.py

    Change-Id: Icd9f9d9ca77068b12227d77469773a325c840001
    Closes-Bug: #1289033
    Co-Authored-By: Kieran Spear <email address hidden>