Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 2 users
Status: Fixed
Owner:
Closed: Oct 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Security



Sign in to add a comment
Security: properly escaped href attribute leading to offline XSS upon saving a page
Reported by inti.de....@gmail.com, Oct 11 2015 Back to list
NOTE: This is a split of  issue #503217  I previously reported

VULNERABILITY DETAILS
Upon saving a page with a crafted anchor tag, escaped HTML characters followed after "#" in the href attribute will render as regular HTML entities. This makes it possible to steal content or CSRF tokens by making a victim save and open a page that includes this link.

VERSION
Chrome Version:  45.0.2454.101 m stable, 48.0.2533.0 canary
Operating System: Windows 10 64 bit

REPRODUCTION CASE

1. Save this page (CTRL + S)
2. Open
3. A popup box will appear


- Link used for this PoC:
http://www.example.com/#"><script>alert(0)</script>

ATTACK SCENARIO

I found a way to abuse this behavior on Facebook:

1. Attacker posts: "Share this post, save the page and see what happens!"
2. Attacker commented the following link: http://www.example.com/#"><script>alert('Thanks,'+document.getElementsByClassName('_2dpb')[0].innerText)</script>a
3. Upon opening the saved page, the victim will see a popup box containing "Thanks", followed by his or hers personal name. Obviously we can steal a lot more interesting stuff other than their name.

I added some screenshots of the PoC in actions (PoC is not made public)



 
href1.PNG
38.0 KB View Download
href2.PNG
12.3 KB View Download
Sorry, forgot to add the link to the actual PoC. My bad!
http://ceukelai.re/href/

But this may a well work
http://www.google.com#"><script>alert(0)</script>
Comment 2 by och...@chromium.org, Oct 12 2015
Labels: Security_Impact-Stable Cr-Blink-Editing Security_Severity-Medium
Owner: yosin@chromium.org
Status: Assigned
Thanks for the report.

Assigning yosin. yosin, please take a look or find a more appropriate owner.
Comment 3 by och...@chromium.org, Oct 12 2015
Labels: M-46
Project Member Comment 4 by clusterf...@chromium.org, Oct 12 2015
Labels: Pri-1
Comment 5 by yosin@chromium.org, Oct 13 2015
Ctrl+C/execCommand("copy") works correctly.
Comment 6 by yosin@chromium.org, Oct 13 2015
It is caused by WebPageSerializerImpl::openTagToString() which append URL parameter
without escaping:

            if (element->hasLegalLinkAttribute(attrName)) {
                // For links start with "javascript:", we do not change it.
                if (attrValue.startsWith("javascript:", TextCaseInsensitive)) {
DANGER~~~~~~~~~~~~~> result.append(attrValue);
                } else {
                    // Get the absolute link
                    WebLocalFrameImpl* subFrame = WebLocalFrameImpl::fromFrameOwnerElement(element);
                    String completeURL = subFrame ? subFrame->frame()->document()->url() :
                                                    param->document->completeURL(attrValue);
                    // Check whether we have local files for those link.
                    if (m_localLinks.contains(completeURL)) {
                        if (!param->directoryName.isEmpty()) {
                            result.appendLiteral("./");
                            result.append(param->directoryName);
                            result.append('/');
                        }
DANGER ~~~~~~~~~~~~~~~~> result.append(m_localLinks.get(completeURL));
                    } else {
HERE ~~~~~~~~~~~~~~~~~~> result.append(completeURL);
                    }
                }

Comment 7 by yosin@chromium.org, Oct 13 2015
Cc: tkent@chromium.org
Status: Started
In review: http://crrev.com/1398453005

Note: other than having URL attribute value, WebPageSerializerImpl escapes them:
  if (param->isHTMLDocument)
    result.append(m_htmlEntities.convertEntitiesInString(attrValue));
  else
    result.append(m_xmlEntities.convertEntitiesInString(attrValue));

Project Member Comment 8 by bugdroid1@chromium.org, Oct 13 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b770d85e37b2d0e248f04cf20606a2f3871ef039

commit b770d85e37b2d0e248f04cf20606a2f3871ef039
Author: yosin <yosin@chromium.org>
Date: Tue Oct 13 10:00:14 2015

Make WebPageSerializerImpl to escape URL attribute values in result.

This patch makes |WebPageSerializerImpl| to escape URL attribute values rather
than directly output URL attribute values into result.

BUG= 542054 
TEST=webkit_unit_tests --gtest_filter=WebPageSerializerTest.URLAttributeValues

Review URL: https://codereview.chromium.org/1398453005

Cr-Commit-Position: refs/heads/master@{#353712}

[modify] http://crrev.com/b770d85e37b2d0e248f04cf20606a2f3871ef039/third_party/WebKit/Source/web/WebPageSerializerImpl.cpp
[modify] http://crrev.com/b770d85e37b2d0e248f04cf20606a2f3871ef039/third_party/WebKit/Source/web/tests/WebPageSerializerTest.cpp
[add] http://crrev.com/b770d85e37b2d0e248f04cf20606a2f3871ef039/third_party/WebKit/Source/web/tests/data/pageserialization/url_attribute_values.html

Comment 9 by aarya@google.com, Oct 13 2015
Status: Fixed
Project Member Comment 10 by clusterf...@chromium.org, Oct 13 2015
Labels: -Restrict-View-SecurityTeam Merge-Triage M-47 Restrict-View-SecurityNotify
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz
Cc: timwillis@chromium.org
Labels: -M-46 -Merge-Triage Merge-Request-47
Requesting Merge for M47 (post-stable patch)
Comment 12 by tin...@google.com, Nov 28 2015
Labels: -Merge-Request-47 Merge-Review-47 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before stable on M47, manual review required.
Labels: reward-topanel
Let's take this to the panel as a separate issue from  Issue 503217 
Cool! You can credit me for both issues as "Inti De Ceukelaire" (plus @securinti if we can add our tweet handle). Thanks for the fixes!
Labels: OS-Windows
Adding OS label, please change if incorrect
Labels: -Merge-Review-47 -Hotlist-Merge-Review Merge-Approved-47 Hotlist-Merge-Approved
Merge approved for M47 (branch 2526)
Project Member Comment 17 by bugdroid1@chromium.org, Dec 2 2015
Labels: -Merge-Approved-47 merge-merged-2526
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/947b9d85a5e06ad7f3d658682db39443d2028c08

commit 947b9d85a5e06ad7f3d658682db39443d2028c08
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Wed Dec 02 08:43:46 2015

Make WebPageSerializerImpl to escape URL attribute values in result.

This patch makes |WebPageSerializerImpl| to escape URL attribute values rather
than directly output URL attribute values into result.

BUG= 542054 
TEST=webkit_unit_tests --gtest_filter=WebPageSerializerTest.URLAttributeValues

Review URL: https://codereview.chromium.org/1398453005

Cr-Commit-Position: refs/heads/master@{#353712}
(cherry picked from commit b770d85e37b2d0e248f04cf20606a2f3871ef039)

Review URL: https://codereview.chromium.org/1487273003 .

Cr-Commit-Position: refs/branch-heads/2526@{#496}
Cr-Branched-From: cb947c0153db0ec02a8abbcb3ca086d88bf6006f-refs/heads/master@{#352221}

[modify] http://crrev.com/947b9d85a5e06ad7f3d658682db39443d2028c08/third_party/WebKit/Source/web/WebPageSerializerImpl.cpp
[modify] http://crrev.com/947b9d85a5e06ad7f3d658682db39443d2028c08/third_party/WebKit/Source/web/tests/WebPageSerializerTest.cpp
[add] http://crrev.com/947b9d85a5e06ad7f3d658682db39443d2028c08/third_party/WebKit/Source/web/tests/data/pageserialization/url_attribute_values.html

Labels: Release-1-M47
Labels: -reward-topanel reward-unpaid reward-500 CVE-2015-6790
Congrats Inte - $500 for this report. There's a patch to M47 that is likely to roll out tomorrow that this fix will ship with. We'll credit you in the release notes and the CVE ID is CVE-2015-6790. 
Labels: -reward-unpaid reward-inprocess
Labels: -reward-inprocess
Project Member Comment 22 by clusterf...@chromium.org, Jan 19 2016
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member Comment 23 by sheriffbot@chromium.org, Oct 1 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 24 by sheriffbot@chromium.org, Oct 2 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: allpublic
Sign in to add a comment