[OSSA-2017-003] XSS in federation mappings UI (CVE-2017-7400)

[Eric Brown]

Found in Mitaka

Steps:
- Setup federation in keystone and horizon
- Launch and login to horizon as an admin
- Click on the Federation->Mappings tab
- Create or update a mapping with the following content that contains javascript

[
    {
        "local": [
            {
                "domain": {
                    "name": "Default"
                },
                "group": {
                    "domain": {
                        "name": "Default"
                    },
                    "name": "Federated Users"
                },
                "user": {
                    "name": "{<script>alert('test');</script>}",
                    "email": "{1}"
                },
                "groups": "{2}"
            }
        ],
        "remote": [
            {
                "type": "REMOTE_USER"
            },
            {
                "type": "MELLON_userEmail"
            },
            {
                "type": "MELLON_groups"
            }
        ]
    }
]

Now whenever this Federation->Mapping page is shown, the javascript will execute.

It appears other pages in horizon protect against such attacks (such as Users, Groups, etc). So I'm guessing that the rendering of this page just needs to be escaped to ignore tags.

[Morgan Fainberg (mdrnstm)]

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.

[Richard Jones (r1chardj0n3s)]

Just a note: to reproduce this you only need to enable federation in Horizon.

[Richard Jones (r1chardj0n3s)]

The fix is quite simple:

diff --git a/openstack_dashboard/dashboards/identity/mappings/tables.py b/openstack_dashboard/dashboards/identity/mappings/tables.py
index df6e8f3..b8c7154 100644
--- a/openstack_dashboard/dashboards/identity/mappings/tables.py
+++ b/openstack_dashboard/dashboards/identity/mappings/tables.py
@@ -75,7 +75,7 @@ def get_rules_as_json(mapping):
     rules = getattr(mapping, 'rules', None)
     if rules:
         rules = json.dumps(rules, indent=4)
- return safestring.mark_safe(rules)
+ return rules

 class MappingsTable(tables.DataTable):

[Morgan Fainberg (mdrnstm)]

While the scope of this is pretty narrow (the cloud admin needs to create a malicious mapping, and iirc only a cloud admin can see this), meaning it is not a practical exploit, it is likely a "Class A" vulnerability that will need to be fixed in all supported branches[1]. I am looking at this as a "Class A" because if a malicious mapping rule is created, it could result in actual exploitation of the XSS bug.

Horizon-Coresec, please confirm the affected versions and let me know if you concur with my assessment. If it is deemed to be completely impractical to exploit by the horizon coresec team, it could be classified as a "C1"[1].

Once the assessment is confirmed, we can continue with the process. I agree the fix is extremely simple (as indicated in #3).

[1] https://security.openstack.org/vmt-process.html#incident-report-taxonomy

[Rob Cresswell (robcresswell-deactivatedaccount)]

The scope is very narrow here. The mappings list is admin only by default, so in most situations I would imagine this could only be implemented by admins and only affect other admins. It could be used maliciously, but frankly I'd imagine if an admin wanted to break something in your cloud, there are easier ways.

That said, the fix is so trivial that I'm happy to backport to all supported releases. Thoughts?

[Rob Cresswell (robcresswell-deactivatedaccount)]

Default policy for that page: https://github.com/openstack/horizon/blob/master/openstack_dashboard/conf/keystone_policy.json#L162

[Jeremy Stanley (fungi)]

If it's only enabling admin-on-admin violence, I don't think we need to work this under embargo. If it's possible for a less-privileged admin to leverage this for effective privilege escalation to a different and more-privileged admin account then I can see issuing an official advisory, but it doesn't seem like we need the added weight of solving this under secrecy and should switch the report to public security.

[Rob Cresswell (robcresswell-deactivatedaccount)]

Richard, when you're ready, I think you can propose a patch and merge right away.

[Jeremy Stanley (fungi)]

In that case I'll switch this to public security so discussion of whether or not we need an advisory won't block pushing a fix.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

commit a835dbfbaa2c70329c08d4b8429d49315dc6d651
Author: Richard Jones <email address hidden>
Date: Tue Mar 7 16:55:39 2017 +1100

    Remove dangerous safestring declaration

    This declaration allows XSS content through the JSON and
    is unnecessary for correct rendering of the content anyway.

    Change-Id: I82355b37108609ae573237424e528aab86a24efc
    Closes-Bug: 1667086

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/442453

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/newton
Review: https://review.openstack.org/442454

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/mitaka
Review: https://review.openstack.org/442455

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/442455
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=ce80bb6fec3cb0262728e7ae8b9d695cf832e5bf
Submitter: Jenkins
Branch: stable/mitaka

commit ce80bb6fec3cb0262728e7ae8b9d695cf832e5bf
Author: Richard Jones <email address hidden>
Date: Tue Mar 7 16:55:39 2017 +1100

    Remove dangerous safestring declaration

    This declaration allows XSS content through the JSON and
    is unnecessary for correct rendering of the content anyway.

    Change-Id: I82355b37108609ae573237424e528aab86a24efc
    Closes-Bug: 1667086
    (cherry picked from commit a835dbfbaa2c70329c08d4b8429d49315dc6d651)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/442454
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=511b325b45b6bd7a88bb6df1a4639b80d0121277
Submitter: Jenkins
Branch: stable/newton

commit 511b325b45b6bd7a88bb6df1a4639b80d0121277
Author: Richard Jones <email address hidden>
Date: Tue Mar 7 16:55:39 2017 +1100

    Remove dangerous safestring declaration

    This declaration allows XSS content through the JSON and
    is unnecessary for correct rendering of the content anyway.

    Change-Id: I82355b37108609ae573237424e528aab86a24efc
    Closes-Bug: 1667086
    (cherry picked from commit a835dbfbaa2c70329c08d4b8429d49315dc6d651)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/442453
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=1407cfe53144146b29679de21f28c952282043ae
Submitter: Jenkins
Branch: stable/ocata

commit 1407cfe53144146b29679de21f28c952282043ae
Author: Richard Jones <email address hidden>
Date: Tue Mar 7 16:55:39 2017 +1100

    Remove dangerous safestring declaration

    This declaration allows XSS content through the JSON and
    is unnecessary for correct rendering of the content anyway.

    Change-Id: I82355b37108609ae573237424e528aab86a24efc
    Closes-Bug: 1667086
    (cherry picked from commit a835dbfbaa2c70329c08d4b8429d49315dc6d651)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/horizon 10.0.3 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/horizon 9.1.2 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/horizon 11.0.1 release.

[Tristan Cacqueray (tristan-cacqueray)]

Bugs that are only exposed to admin usually do not warrant an advisory, but since this is a stored XSS, it could use an OSSA... What do you think?

[Jeremy Stanley (fungi)]

Seems low-criticality, but I suppose an advisory is still a reasonable next step on this.

[Tristan Cacqueray (tristan-cacqueray)]

Impact description proposed here: https://review.openstack.org/#/c/447064

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/447064
Committed: https://git.openstack.org/cgit/openstack/ossa/commit/?id=d9fb681d40ed9b2ec535b3ffa49451edfd199167
Submitter: Jenkins
Branch: master

commit d9fb681d40ed9b2ec535b3ffa49451edfd199167
Author: Tristan Cacqueray <email address hidden>
Date: Fri Mar 17 16:49:35 2017 +0000

    Adds OSSA-2017-003 (CVE-2017-7400)

    Change-Id: Iead38e4f72cfe54102612a07a4001862cb5fd32c
    Closes-Bug: #1667086

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/horizon 12.0.0.0b1 development milestone.