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 1 user
Status: Fixed
Owner:
User never visited
Closed: Aug 2011
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
DocumentLoader use after free in KURL::strippedForUseAsReferrer
Reported by miau...@gmail.com, Jul 14 2011 Back to list
VULNERABILITY DETAILS

as per discussion with inferno. 

one day I was able to reproduce the 456 inside 3176 bug in a browser with valgrind, but otherwise these are reproducing in DumpRenderTree compiled with asan. 

stack is either:

Reply: ==7030== ERROR: AddressSanitizer crashed on address 0x00007fffc30b92c8 at pc 0x1806482 bp 0x7fffffffa0b0 sp 0x7fffffff9f20
READ of size 1 at 0x00007fffc30b92c8 thread T0
0x00007fffc30b92c8 is located 456 bytes inside of 3176-byte region [0x00007fffc30b9100,0x00007fffc30b9d68)
freed by thread T0 here:
previously allocated by thread T0 here:
Stats: 0M freed by 0 calls
Stats: 0M really freed by 0 calls
	WebCore::KURL::strippedForUseAsReferrer() [0x1806482]
	WebCore::FrameLoader::setOutgoingReferrer() [0x21b9177]
	WebCore::DocumentWriter::begin() [0x21a153b]
	WebCore::DocumentWriter::replaceDocument() [0x21a09e5]
	WebCore::ScriptController::executeIfJavaScriptURL() [0x1aac187]
	WebCore::SubframeLoader::requestFrame() [0x221ea8a]

or 

Reply: ==20114== ERROR: AddressSanitizer crashed on address 0x00007fffc359a308 at pc 0xd5607d bp 0x7fffffff90f0 sp 0x7fffffff90b0
READ of size 8 at 0x00007fffc359a308 thread T0
0x00007fffc359a308 is located 8 bytes inside of 16-byte region [0x00007fffc359a300,0x00007fffc359a310)
freed by thread T4 here:
previously allocated by thread T4 here:
Thread T4 created by T0 here:
Stats: 0M freed by 0 calls
Stats: 0M really freed by 0 calls
	WebCore::KURLGooglePrivate::init<>() [0x180bfdb]
	WebCore::KURL::KURL() [0x180b7ca]
	WebCore::blankURL() [0x18067a7]
	WebCore::Document::completeURL() [0x1d5b190]
	WebCore::HTMLLinkElement::parseMappedAttribute() [0x161d4ec]
	WebCore::StyledElement::attributeChanged() [0x1e78858]
	WebCore::Element::setAttributeMap() [0x1da449f]


with varying offset and size.

VERSION
Chrome Version: DumpRenderTree on trunk compiled with asan
Operating System: linux 64 bit

REPRODUCTION CASE

attached a bunch of files and DumpRenderTree logs.

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: drt


 
kurl.zip
309 KB Download
Owner: security@chromium.org
Status: Available
Summary: DocumentLoader related use-after-frees (was: NULL)
This should be definitely fixed in browser, dont care about DumpRenderTree.

I just cced you. http://code.google.com/p/chromium/issues/detail?id=84946. I found this as well. 

Do a fresh checkout (gclient revert, rm -rf ./out) and please verify which ones are left. 

Labels: -Pri-0 -Area-Undefined Pri-1 Area-WebKit SecSeverity-High OS-All Mstone-13
Looks like the fix from Oliver is not enough, miaubiz can still repro it.
Comment 3 by miau...@gmail.com, Jul 14 2011
here's a repro vg log coming up in just a minute
118.html
13.5 KB View Download
Comment 4 by miau...@gmail.com, Jul 14 2011

Google Chrome	14.0.814.0 (Official Build 91661) dev
OS	Linux
WebKit	535.1 (trunk@90501)


vg-google-chrome-unstable-89330.txt
36.9 KB View Download
Comment 5 by abarth@chromium.org, Jul 14 2011
Cc: japhet@chromium.org
japhet is this the same HTMLLinkElement bug, or is this one different?
Comment 6 by japhet@chromium.org, Jul 14 2011
abarth, if you mean http://code.google.com/p/chromium/issues/detail?id=87593, I'd say the odds are pretty small that they're the same root issue, but I've been wrong lots of times before.
Comment 7 by miau...@gmail.com, Jul 16 2011
sorry for not checking back. same situation after rm -rf out and gclient revert, browser doesn't crash with current revisions, but DumpRenderTree is showing the same crashes.
Labels: reward-topanel
Miaubiz, if you make reduced testcases for the 2 crash stacks that crash DumpRenderTree and are valid bugs, that will qualify for a higher reward. DumpRenderTree testcases can be test framework specific, but many times, we have layouttestcontroller specific commands that DumpRenderTree understands but the browser needs manual interaction for that. So, DumpRenderTree might infact be pointing to a real problem that the dev didnt fix properly or is a different problem altogether.
For reduced testcases, you can run ASAN on DumpRenderTree for best reduction.
Comment 10 by miau...@gmail.com, Jul 17 2011
got it reproing in the browser now with the attached file and the attached js-test-post.js in js/resources relative to the html file.
js-test-post.js
233 bytes View Download
yesh.html
47.1 KB View Download
asan.txt
6.3 KB View Download
Nice Miaubiz. Thanks for confirming this reproduces in browser with ASAN.

2 small things
1) ASAN no longer symbolizes by default. Which makes harder to understand your stacks.  Here is a way to give proper ones. Add this ./out/Release/chrome test.html 2>&1 | third_party/asan/scripts/asan_symbolize.py | c++filt
2) I think you have two bugs here in DocumentLoader. From your initial description, it looked like one of them might be fixed. Here is what you can do. Update to latest trunk and see which ones reproduce in DumpRenderTree. I am now not much worried for a browser repro. If you can provide one more comment with reduced repro (DumpRenderTree/Browser) for each of two stacks, you will increase your chances of higher reward and i can seperate the 2nd stack into a new bug (or you can file one yourself). If you are unable to reduce, that is ok, but please let me know and i can start to work on that. I just want rewarded fully for your awesome bug and efforts.
Comment 12 by miau...@gmail.com, Jul 17 2011
I got it down to 25 lines and no external resources or weird shit btw.
Comment 13 by miau...@gmail.com, Jul 17 2011
still working on this but here goes. it sometimes takes like 20 reloads for it to crash asan
back.html
726 bytes View Download
Perfect. 25 lines for 2 different stacks ? or just 1 stack. Please take your time, no hurry. File a new bug if you have the second repro reduced.
Comment 15 by miau...@gmail.com, Jul 17 2011
thanks for that symbols thing
asan-symbols.txt
18.9 KB View Download
Comment 16 by miau...@gmail.com, Jul 17 2011
I'm getting craches with this one all the time now
documentloader.html
726 bytes View Download
Summary: DocumentLoader use after free in KURL::strippedForUseAsReferrer (was: NULL)
Sad that it just hit by our fuzzers. Miaubiz, you are fast and you win :)
A slight more reduction using my minimizer

<div><script>
iframe1 = document.createElement('iframe');
document.body.appendChild(iframe1);
document1 = iframe1.contentDocument.implementation.createHTMLDocument("document");
iframe1 = document.createElement('iframe');
document.body.appendChild(iframe1);
document1 = iframe1.contentDocument.implementation.createHTMLDocument("document");
eval("");
  xyz();
</script>
<script>
  for(var a=0;a<18;a++) {
    iframe1 = document.createElement('iframe');
    iframe1.setAttribute("src", "javascript:''");
    document.body.appendChild(iframe1);
  }
</script>
Comment 19 by mal@google.com, Jul 19 2011
Owner: ----
Comment 20 by kcc@chromium.org, Jul 21 2011
Labels: Stability-AddressSanitizer
Owner: scottmg@chromium.org
Here's another DocumentLoader lifetime issue, Scott. Fancy it?
In fact looking at the repro, it makes me wonder if your recent patch didn't already have some affect here?

The challenge is it's probably difficult to reproduce a crash / error here without ASAN or Valgrind.
Thanks (? :) No promises, as I'm still feeling pretty lost in that code, but I'll at least have a look and see what I can figure out.
Better repro with ClusterFuzz

<iframe src="javascript:''"></iframe>
<a><summary><pre><pre><pre><pre><pre><iframe src="javascript:''"></iframe>

<a>
miaubiz, can you still reproduce this on trunk. I can no longer reproduce this. Seems like something has fixed this, checking with you to make sure that i am right ?
Comment 26 by miau...@gmail.com, Aug 22 2011
I'm still seeing this sporadically, but harder to repro than before.
do you have a repro that still reproduces ?
@miaubiz: we just landed http://trac.webkit.org/changeset/93521, which fixes some lifetime issues in the document loader. Can you still repro it with that change?
Comment 29 by miau...@gmail.com, Aug 22 2011
here's the one that was crashing for earlier build.. 93477.. I did rm -rf out and will get back to you in like an hour when it's done.
456.html
81.8 KB View Download
Comment 30 by miau...@gmail.com, Aug 22 2011
here's the stack for that 456.html with dirty 93477
asan456.txt
10.5 KB View Download
BTW, I doubt Chromium has rolled WebKit to include that revision just yet, as it is very fresh. You could always manually hack your DEPS to point to that revision of WebKit.
Comment 32 by miau...@gmail.com, Aug 22 2011
still there with:

Chromium	15.0.860.0 (Developer Build 97689-dirty)
OS	Linux
WebKit	535.2 (trunk@93523)

and the above repro.

@scarybeasts: :D
Comment 33 by miau...@gmail.com, Aug 22 2011
by which I mean, with webkit git I only have to do 'git co master' or 'git co gclient' to switch between gclient and master.
Cc: -japhet@chromium.org
Labels: -Mstone-13 Mstone-14
Owner: japhet@chromium.org
@japhet: seems this is a lifetime issue in a similar area to the other recent fix. Mind taking a look?
Could I ask someone else to take this bug?  I don't have a linux box, so it's going to be tough for me to reproduce a bug that requires asan.

Sorry :(
Owner: abarth@chromium.org
Adam expressed an interest :)

FWIW, I would expect valgrind to catch something like this easily too. Not sure how good our Mac / Valgrind environment is?

Anyway, given to Adam.
Comment 37 by miau...@gmail.com, Aug 25 2011
@scarybeasts: there's so many frames being destroyed and created that valgrind hates it and just gives a million lines of v8 jit :(

vex amd64->IR: unhandled instruction bytes: 0xFF 0xFF 0x1 0x0 0x0 0x0 0x74 0x0
==4879== valgrind: Unrecognised instruction at address 0x2a9ace470e46.
==4879==    at 0x2A9ACE470E46: ???
==4879==    by 0x2A9ACE470C17: ???
==4879==    by 0x2A9ACE45E3A7: ???
==4879==    by 0x2A9ACE45B404: ???
==4879==    by 0x2A9ACE46AF70: ???
==4879==    by 0x2A9ACE46ABF4: ???
==4879==    by 0x2A9ACE46B7E7: ???
==4879==    by 0x2A9ACE46ACBE: ???
==4879==    by 0x2A9ACE45124D: ???
==4879==    by 0x2A9ACE46B151: ???
==4879==    by 0x2A9ACE46ABF4: ???
==4879==    by 0x2A9ACE45124D: ???
==4879==    by 0x2A9ACE467300: ???
==4879==    by 0x2A9ACE455000: ???
==4879==    by 0x11A81AA: v8::internal::Invoke(bool, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Object***, bool*) (in /usr/lib/chromium-browser/chromium-browser)

<beg/>
Adam, if we can get an unrisky change landed by Weds, we can get this fix into the final Beta.
pls don't beg.  /me looks now.
I understand it.  Great bug.
@scarybeast: The patch is very non-risky and posted upstream for review.
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify Merge-Approved
Status: FixUnreleased
WoW!! Right on time. http://trac.webkit.org/changeset/94112
I aim to please.
Labels: -Merge-Approved Merge-Merged
Merged to M14: http://trac.webkit.org/changeset/94125
Labels: -reward-topanel reward-1000 reward-unpaid
@miaubiz: nice find, thanks for taking the trouble to work on the big repro and minimize it down. Certainly that helps qualify it for a $1000 Chromium Security Reward :)

----
Boilerplate text:
Please do NOT publicly disclose details until a fix has been released to all our
users. Early public disclosure may cancel the provisional reward.
Also, please be considerate about disclosure when the bug affects a core library
that may be used by other products.
Please do NOT share this information with third parties who are not directly
involved in fixing the bug. Doing so may cancel the provisional reward.
Please be honest if you have already disclosed anything publicly or to third parties.
----
Labels: CVE-2011-2847
Labels: -reward-unpaid
Payment in system.
Labels: SecImpacts-Stable
Batch update.
Comment 49 by cdn@chromium.org, May 15 2012
Status: Fixed
Marking old security bugs Fixed..
Project Member Comment 50 by bugdroid1@chromium.org, Oct 13 2012
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member Comment 51 by bugdroid1@chromium.org, Mar 10 2013
Labels: -Type-Security -Area-WebKit -SecSeverity-High -Mstone-14 -Stability-AddressSanitizer -SecImpacts-Stable Security-Impact-Stable Cr-Content Security-Severity-High Performance-Memory-AddressSanitizer Type-Bug-Security M-14
Project Member Comment 52 by bugdroid1@chromium.org, Mar 13 2013
Labels: Restrict-View-EditIssue
Project Member Comment 53 by bugdroid1@chromium.org, Mar 13 2013
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Labels: -Restrict-View-SecurityNotify -Restrict-View-EditIssue
Project Member Comment 55 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Severity-High Security_Severity-High
Project Member Comment 56 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member Comment 57 by bugdroid1@chromium.org, Apr 1 2013
Labels: -Performance-Memory-AddressSanitizer Stability-Memory-AddressSanitizer
Project Member Comment 58 by bugdroid1@chromium.org, Apr 6 2013
Labels: -Cr-Content Cr-Blink
Project Member Comment 59 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 60 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