[OSSA 2015-010] help_text parameter of fields is vulnerable to arbitrary html injection (CVE-2015-3219)

[Nikita Konovalov]

The Field class help_text attribute is vulnerable to code injection if the text is somehow taken from the user input.

Heat UI allows to create stacks from the user input which define parameters. Those parameters are then converted to the input field which are vulnerable.

The heat stack example exploit:

description: Does not matter
heat_template_version: '2013-05-23'
outputs: {}
parameters:
  param1:
    type: string
    label: normal_label
    description: hack="><script>alert('YOUR HORIZON IS PWNED')</script>"
resources: {}

[Tristan Cacqueray (tristan-cacqueray)]

Since this report concerns a possible security risk, an incomplete security advisory task has been added while the core security reviewers for the affected project or projects confirm the bug and discuss the scope of any vulnerability along with potential solutions.

[Jeremy Stanley (fungi)]

Is the implication here that use of untrusted Heat stacks is normally considered safe practice?

[Jeremy Stanley (fungi)]

Oh, I see. The risk is exposed in Horizon itself, that a malicious user could create such a stack and then convince another user or administrator to browse it through the UI triggering possible cross-site scripting attacks, et cetera.

[Nikita Konovalov (nkonovalov)]

The stack templates may also be downloaded from the insecure resource via http and if there is a MitM attack going on on that resource the heat template using this exploit may be obtained.

[Nikita Konovalov (nkonovalov)]

Django security team has pointed out that it's a documented behavior on the Django side.
https://docs.djangoproject.com/en/1.8/ref/models/fields/#help-text

So it looks like that Horizon should care about escaping the values for the help_text.

[Tristan Cacqueray (tristan-cacqueray)]

At least Juno and Kilo are affected, is Icehouse also affected ?

Assuming the description field is only rendered during the stack creation and there is no other ways to make it's content executed (like through a show_info url), here is the impact description draft:

Title: XSS in Horizon Heat stack creation
Reporter: Nikita Konovalov (Mirantis)
Products: Horizon
Affects: 2014.2 versions through 2014.2.3 and version 2015.1.0

Description:
Nikita Konovalov from Mirantis reported a vulnerability in Horizon. 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 during the stack creation. It may result in potential assets theft like user access credentials. Only setups exposing the orchestration dashboard in Horizon are affected.

[Tristan Cacqueray (tristan-cacqueray)]

@lhcheng can you please triage this bug ?

[Grant Murphy (gmurphy)]

+1 impact description

[Lin Hua Cheng (lin-hua-cheng)]

Attaching patch to clean the descrip value before rendering

[David Lyle (david-lyle)]

+1 to the patch in #9. Thanks Lin!

[Matthias Runge (mrunge)]

Yes, patch in #9 looks sane to me. Thank you!

[Tristan Cacqueray (tristan-cacqueray)]

The patch in #9 applies cleanly on Juno too.

The vulnerable code seems also present in Icehouse but I couldn't reproduce the bug there, can someone please check if Icehouse is also affected ?

[Lin Hua Cheng (lin-hua-cheng)]

Tristan: Icehouse is not affected, this is because the help_text is always escaped on HTML template.

Juno - assume that help text is always safe:
        <span class="help-icon" data-toggle="tooltip"
              data-placement="top" title="{{ field.help_text|safe }}">

Icehouse - always escape the text:
        <span class="help-block">{% filter force_escape %} {{ field.help_text }} {% endfilter %} </span>

[Tristan Cacqueray (tristan-cacqueray)]

Proposed disclosure date:
2015-06-09, 1500UTC

[Jeremy Stanley (fungi)]

Tristan's impact description in comment #6 looks good to me, and I agree that the disclosure schedule in comment #14 is reasonable.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/189820

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/juno
Review: https://review.openstack.org/189821

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/kilo
Review: https://review.openstack.org/189822

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/189822
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=84da479739fbfff75d9492d00fd0ed1a31ea52b3
Submitter: Jenkins
Branch: stable/kilo

commit 84da479739fbfff75d9492d00fd0ed1a31ea52b3
Author: lin-hua-cheng <email address hidden>
Date: Mon Jun 1 17:55:00 2015 -0700

    Escape the description param from heat template

    The heat template allows user to define custom parameters,
    the fields are then converted to input fields. The description
    param maps to the help_text attribute of the field.

    Since the value comes from the user, the value must be escaped
    before rendering.

    Co-Authored-By: Lin Hua Cheng <email address hidden>
    Change-Id: I79d540a8363b2507c4bccdc0cc38e283962919d2
    Closes-bug: #1453074

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/189821
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=dab92e7d2f576caea8f81c8e22a516fb45633794
Submitter: Jenkins
Branch: stable/juno

commit dab92e7d2f576caea8f81c8e22a516fb45633794
Author: lin-hua-cheng <email address hidden>
Date: Mon Jun 1 17:55:00 2015 -0700

    Escape the description param from heat template

    The heat template allows user to define custom parameters,
    the fields are then converted to input fields. The description
    param maps to the help_text attribute of the field.

    Since the value comes from the user, the value must be escaped
    before rendering.

    Co-Authored-By: Lin Hua Cheng <email address hidden>
    Change-Id: I79d540a8363b2507c4bccdc0cc38e283962919d2
    Closes-bug: #1453074

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/189985

[OpenStack Infra (hudson-openstack)]

Change abandoned by Ian Cordasco (<email address hidden>) on branch: stable/icehouse
Review: https://review.openstack.org/189985
Reason: https://github.com/openstack/horizon/blob/78b6f5ed6145dcb2e9ce2caed0c87d159c12697b/horizon/templates/horizon/common/_form_fields.html#L27 shows that this isn't a problem because the template escapes this. The notice was misleading in not excluding Icehouse from the list of affected versions.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/189820
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=28d8f49786d6df297b2574514916fa782e941e09
Submitter: Jenkins
Branch: master

commit 28d8f49786d6df297b2574514916fa782e941e09
Author: lin-hua-cheng <email address hidden>
Date: Mon Jun 1 17:55:00 2015 -0700

    Escape the description param from heat template

    The heat template allows user to define custom parameters,
    the fields are then converted to input fields. The description
    param maps to the help_text attribute of the field.

    Since the value comes from the user, the value must be escaped
    before rendering.

    Co-Authored-By: Lin Hua Cheng <email address hidden>
    Change-Id: I79d540a8363b2507c4bccdc0cc38e283962919d2
    Closes-bug: #1453074

[Tristan Cacqueray (tristan-cacqueray)]

All patches are now merged, shouldn't series task be added to Horizon ?