My favorites | Sign in
Project Home Downloads Wiki Issues Code Search
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 42736: Memory corruption (read random system memory) or crash
3 people starred this issue and may be notified of changes. Back to list
 
Reported by Michail....@gmail.com, Apr 28, 2010
It is possible to crash browser or read some memory using special page.
Check details (in attach). HTML file (+js) and instructions in it.

Thanks a lot.


4.1.249.1064 (45376), Windows XP 32 bit, SP3
google_bug.zip
204 KB   Download
Apr 28, 2010
#1 scarybea...@gmail.com
Sounds interesting!
Looks OK on my Linux dev channel build.

Justin or Abhishek - do you happen to have a Windows stable version around to check 
this on?
Cc: jsc...@chromium.org infe...@chromium.org
Apr 28, 2010
#2 infe...@chromium.org
trying to reproduce bug on my windows.
Apr 28, 2010
#3 scarybea...@gmail.com
Actually, this sounds like it needs a server running? Clicking on 1 or 2 seem to hit 
http://localhost Do you have a self-contained version that runs only on the 
filesystem?
Apr 28, 2010
#4 Michail....@gmail.com
No, you not need a server.
I'll create HTML with crash - one minute.
Apr 28, 2010
#5 Michail....@gmail.com
Also, checked on 2 computers - reproduces.
Apr 28, 2010
#6 infe...@chromium.org
I can replicate the crash 5.0.388.0 (45605) windows. investigating more (my debugger
is giving some weird issues, fixing it first to analyze furthur).
http://infernohacks.com/t/a/Syntax%20analyzer%20demo.htm

Status: Assigned
Owner: infe...@chromium.org
Cc: -infe...@chromium.org
Apr 28, 2010
#7 infe...@chromium.org
This is not a browser crash, but a renderer crash. I will post more analysis soon.
Apr 28, 2010
#8 jsc...@chromium.org
There's definitely an OOB read. I got a sad tab outside of the debugger, but not 
inside. However, running a debug build hits the following ASSERT in 
WebCore::getFallbackFamily() from FontUtilsChromiumWin.cpp:

    ASSERT(characters && characters[0] && length > 0);

The ASSERT is triggered because characters[0] == 0 in my testing. Here's part of the 
stack:

WebCore::getFallbackFamily(const wchar_t * characters=0x175cb8fe, int length=1, 
WebCore::FontDescription::GenericFamilyType generic=StandardFamily, int * 
charChecked=0x00000000, UScriptCode * scriptChecked=0x00000000)  Line 247 + 0x32 
bytes	C++
WebCore::UniscribeHelper::shape(const wchar_t * input=0x175cb8fe, int itemLength=1, 
int numGlyphs=32, tag_SCRIPT_ITEM & run={...}, WebCore::UniscribeHelper::Shaping & 
shaping={...})  Line 633 + 0x13 bytes	C++
WebCore::UniscribeHelper::fillShapes()  Line 728 + 0x34 bytes	C++
WebCore::UniscribeHelper::initWithOptionalLengthProtection(bool 
lengthProtection=true)  Line 145	C++
WebCore::UniscribeHelper::init()  Line 165	C++
WebCore::UniscribeHelperTextRun::UniscribeHelperTextRun(const WebCore::TextRun & 
run={...}, const WebCore::Font & font={...})  Line 59	C++
WebCore::Font::floatWidthForComplexText(const WebCore::TextRun & run={...}, 
WTF::HashSet<WebCore::SimpleFontData const *,WTF::PtrHash<WebCore::SimpleFontData 
const *>,WTF::HashTraits<WebCore::SimpleFontData const *> > * __formal=0x00000000, 
WTF::HashSet<WebCore::SimpleFontData const *,WTF::PtrHash<WebCore::SimpleFontData 
const *>,WTF::HashTraits<WebCore::SimpleFontData const *> > * __formal=0x00000000)  
Line 509	C++
WebCore::Font::floatWidth(const WebCore::TextRun & run={...}, 
WTF::HashSet<WebCore::SimpleFontData const *,WTF::PtrHash<WebCore::SimpleFontData 
const *>,WTF::HashTraits<WebCore::SimpleFontData const *> > * 
fallbackFonts=0x00000000, WebCore::GlyphOverflow * glyphOverflow=0x00000000)  Line 
202	C++
WebCore::Font::width(const WebCore::TextRun & run={...}, 
WTF::HashSet<WebCore::SimpleFontData const *,WTF::PtrHash<WebCore::SimpleFontData 
const *>,WTF::HashTraits<WebCore::SimpleFontData const *> > * 
fallbackFonts=0x00000000, WebCore::GlyphOverflow * glyphOverflow=0x00000000)  Line 97 
+ 0x22 bytes	C++
WebCore::textWidth(WebCore::RenderText * text=0x0086cd8c, unsigned int from=443, 
unsigned int len=325, const WebCore::Font & font={...}, int xPos=0, bool 
isFixedPitch=false, bool collapseWhiteSpace=true)  Line 1302	C++
WebCore::RenderBlock::findNextLineBreak(WebCore::BidiResolver<WebCore::InlineIterator
,WebCore::BidiRun> & resolver={...}, bool firstLine=false, bool & isLineEmpty=false, 
bool & previousLineBrokeCleanly=false, WebCore::EClear * clear=0x0636cd30)  Line 1625 
+ 0x3b bytes	C++
WebCore::RenderBlock::layoutInlineChildren(bool relayoutChildren=false, int & 
repaintTop=114, int & repaintBottom=263)  Line 671 + 0x33 bytes	C++
WebCore::RenderBlock::layoutBlock(bool relayoutChildren=false)  Line 745	C++
WebCore::RenderBlock::layout()  Line 670 + 0x14 bytes	C++

Apr 28, 2010
#9 Michail....@gmail.com

                        var last = 1;
                        function doCrash() {
                            if (last == 1) {
                                var a = (Math.random() * 31) + 2;
                                a = a - (a % 1);
                                eval("markInText" + a +"()");
                                last = 0;
                            } else {
                                markInText1();
                                last = 1;
                            }
                            return 0;
                        }
                        function crash() {
                            doCrash();
                            setTimeout("crash()", 100);                            
                        }


You can execute add it in page to crash browser in fast way. Run crash() - wait for 10 sec. If there 
is no crash - reload page and try again.
Apr 28, 2010
#10 infe...@chromium.org
Jungshik, can you take a look at this one ?
Owner: js...@chromium.org
Cc: -jsc...@chromium.org dglaz...@chromium.org
Labels: -Area-Undefined Area-WebKit
Apr 28, 2010
#11 scarybea...@gmail.com
Great bug. Since the OOB memory is retrieveable via 
document.getElementById('text').innerText[0...100] then this is at least a 
SecSeverity-Medium
Labels: SecSeverity-Medium
Apr 28, 2010
#12 infe...@chromium.org
(No comment was entered for this change.)
Labels: WebKit-Core Mstone-5
May 2, 2010
#13 infe...@chromium.org
Jungshik, did you get a chance to look at it ? Or, can you please advise a suitable
owner for this bug.
May 3, 2010
#15 jshin@chromium.org
I'm sorry I mised this. I'll take a look. 

BTW,  have an unrelated CL pending review touching the same file. 


May 3, 2010
#16 infe...@chromium.org
Thanks Jungshik for helping with this. 
May 11, 2010
#17 jsc...@chromium.org
Jshin, any progress on this? It's currently flagged m5 and the window is about to 
close.

May 11, 2010
#18 jshin@chromium.org
Sorry. I'm hard time reproducing it. I'll try more and come up with a fix tonight. 

May 11, 2010
#19 jshin@chromium.org
The buffer appears to be corrupted well before it reaches the complex script 
measuring code (Uniscribe). As early as WebCore::RenderBlock::findNextLineBreak (if 
not earlier), the buffer is 'corrupt'. 

In that frame, both |o| (RenderObject pointer) and |t| (RenderText pointer) points to 
"<14 SP's>Window Vi渀گH" (26 chars long). "Windows Vista" is replaced by "Window Vi
渀گH". 

In the previous frame(RenderBlock::layoutInlineChildren), |resolver| appears to be 
init'd with a corrupt value. Perhaps, the actual corruption happens much earlier. 
|resolver|'s position is much larger than the length of the buffer. As a result, 
|len| in the following code in the next frame is set to a huge value (because strlen 
<< pos)

           int strlen = t->textLength();
           int len = strlen - pos;
           const UChar* str = t->characters();


I can put on wall-paper in the measuring/drawing code, but it's not a real fix. 

I've just reproduced the problem with Chrome *and* Safari on Mac and Chrome on Linux 
(the text in the gray box gets corrupted the same way as on Windows if I press box #1 
and box #2 and box #1 again). 

Obviously, wall-papering in the measuring/drawing code of Windows doesn't do any good 
for Mac/Linux. This is clearly a Webkit bug (I was afraid it might be a V8 issue, but 
it's not). 

We need a reduced test to expedite the diagnosis. 

I'm really sorry that I didn't get to this much sooner. I'll report this to the 
upstream. 

Status: Available
Owner: ---
Cc: js...@chromium.org
Labels: NeedsReduction OS-All
May 11, 2010
#20 jshin@chromium.org
set to a huge value ==> set to a negative value. |len| is a 'loop variable' in a while 
loop coming later. So, depending on what happens inside the loop, the renderer may not 
crash despite a random memory access. 

May 11, 2010
#21 jshin@chromium.org
Filed a webkit bug ( https://bugs.webkit.org/show_bug.cgi?id=38977 ). I cc'd everyone 
whose email here whose email address I know. 

Cc: aba...@chromium.org
May 20, 2010
#22 jsc...@chromium.org
(No comment was entered for this change.)
Labels: -Pri-0 Pri-2
May 26, 2010
#25 jsc...@chromium.org
The test case doesn't reproduce, and looking at the changeset for the original bug it 
doesn't appear to me that it's the same issue: 
http://trac.webkit.org/changeset?old_path=/&old=17696&new_path=/&new=17697

We've reported the problem upstream to WebKit, and if no one else gets to it soon one 
of the Chrome devs will have another look at it.

May 26, 2010
#26 infe...@chromium.org
I look to have found out the problem. Analyzing more and testing fix.
Owner: infe...@chromium.org
Jun 23, 2010
#27 chromium.cdn@gmail.com
+cdn
Cc: c...@chromium.org
Jun 25, 2010
#28 chromium.cdn@gmail.com
I've got the repro down to 70 or so fairly understandable lines. Strangely though, notice on line 11 inside the load event handler that you have to throw exactly that type of exception.
highlight.html
3.2 KB   View   Download
Jun 25, 2010
#29 chromium.cdn@gmail.com
Forgot to mention the repro involves the following:

1) Splitting text nodes and replacing the split nodes with span element wrapping clones of the split nodes. 
2) Removing the wrapper nodes and replacing them with the wrapped text nodes.
3) Normalizing all the text to coalesce adjoining text nodes.
4) Repeating step one with a larger number of different (non-overlapping) nodes

This can probably be reduced more, but what's actually happening seems to be a lot clearer now.

Jun 25, 2010
#30 chromium.cdn@gmail.com
(No comment was entered for this change.)
Owner: ---
Jun 25, 2010
#31 jsc...@chromium.org
This is starting to look like it may be a duplicate of the replaceChild case from  issue 35366 .

Jun 27, 2010
#32 infe...@chromium.org
(No comment was entered for this change.)
Labels: -NeedsReduction
Jun 28, 2010
#33 chromium.cdn@gmail.com
This was further reduced by mitz upstream

<p id="text" style="width: 200px; outline: solid;">AAAAAAAAsBBBBBBBB AAAAAAAAAA AAAAAAAAAAAAAAA CCC <!-- -->Z</p> 
<script> 
	var paragraph = document.getElementById("text");
    paragraph.removeChild(paragraph.childNodes[1]);
 
    // Force layout
    document.body.offsetTop;
 
    paragraph.normalize();
    paragraph.firstChild.splitText(0);
</script>
Jun 29, 2010
#34 infe...@chromium.org
Fixed in <http://trac.webkit.org/changeset/62134>.
Status: WillMerge
Jun 29, 2010
#35 infe...@chromium.org
(No comment was entered for this change.)
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Jul 2, 2010
#36 scarybea...@gmail.com
(No comment was entered for this change.)
Cc: taviso
Jul 8, 2010
#37 scarybea...@gmail.com
@Michail: congratulations! This bug report provisionally qualifies for a $500 Chromium Security Reward. Although SecSeverity-Medium, we found this to be a particularly interesting bug. We'll ship the fix to users shortly.
Labels: reward-500 reward-unpaid
Jul 9, 2010
#38 Michail....@gmail.com
Wow, thanks!
Waiting for your final decision :)

Jul 14, 2010
#39 bugdroid1@gmail.com
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=52369 

------------------------------------------------------------------------
r52369 | cdn@chromium.org | 2010-07-14 12:00:50 -0700 (Wed, 14 Jul 2010) | 30 lines
Changed paths:
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/375/LayoutTests/fast/text/setData-dirty-lines-expected.checksum
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/375/LayoutTests/fast/text/setData-dirty-lines-expected.png
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/375/LayoutTests/fast/text/setData-dirty-lines-expected.txt
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/375/LayoutTests/fast/text/setData-dirty-lines.html
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/375/LayoutTests/fast/text/splitText-dirty-lines-expected.checksum
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/375/LayoutTests/fast/text/splitText-dirty-lines-expected.png
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/375/LayoutTests/fast/text/splitText-dirty-lines-expected.txt
   A http://src.chromium.org/viewvc/chrome/branches/WebKit/375/LayoutTests/fast/text/splitText-dirty-lines.html
   M http://src.chromium.org/viewvc/chrome/branches/WebKit/375/WebCore/dom/CharacterData.cpp?r1=52369&r2=52368
   M http://src.chromium.org/viewvc/chrome/branches/WebKit/375/WebCore/dom/Text.cpp?r1=52369&r2=52368

Merge 62134 - <rdar://problem/7975842> Certain text is repeated after using splitText()

Reviewed by Darin Adler.

WebCore: 

Tests: fast/text/setData-dirty-lines.html
       fast/text/splitText-dirty-lines.html

* dom/CharacterData.cpp:
(WebCore::CharacterData::setData): Call RenderText::setTextWithOffset() rather than
setText(), because only the former correctly dirties line boxes.
* dom/Text.cpp:
(WebCore::Text::splitText): Ditto.

LayoutTests: 

* fast/text/setData-dirty-lines-expected.checksum: Added.
* fast/text/setData-dirty-lines-expected.png: Added.
* fast/text/setData-dirty-lines-expected.txt: Added.
* fast/text/setData-dirty-lines.html: Added.
* fast/text/splitText-dirty-lines-expected.checksum: Added.
* fast/text/splitText-dirty-lines-expected.png: Added.
* fast/text/splitText-dirty-lines-expected.txt: Added.
* fast/text/splitText-dirty-lines.html: Added.



BUG=42736
Review URL: http://codereview.chromium.org/2927014
------------------------------------------------------------------------

Jul 14, 2010
#40 chromium.cdn@gmail.com
(No comment was entered for this change.)
Status: FixUnreleased
Jul 20, 2010
#41 jimheb...@chromium.org
(No comment was entered for this change.)
Cc: rohi...@chromium.org
Jul 20, 2010
#42 rohi...@chromium.org
Verified on Mac 5.0.375.121 (Official Build 52864) beta

Aug 2, 2010
#43 Michail....@gmail.com
Cool,looks fixed on 5.0.375.125 Windows XP.
But why "FixUnreleased"?

Aug 2, 2010
#44 scarybea...@gmail.com
Yeah, the name FixUnreleased is a little strange but it basically means that we haven't yet opened up the bug to public view.
In this case, it's not clear whether Safari has fixed this yet or not. We generally don't want to open up a bug to public view until Safari is safe.
Aug 2, 2010
#45 scarybea...@gmail.com
@Michail: e-mail me at cevans@chromium.org for steps on how to collect your reward.
Mar 21, 2011
#46 jsc...@chromium.org
(No comment was entered for this change.)
Labels: Type-Security
Oct 4, 2011
#47 jsc...@chromium.org
Batch update.
Labels: SecImpacts-Stable
Apr 18, 2012
#48 jsc...@chromium.org
Lifting view restrictions.
Labels: -Restrict-View-SecurityNotify
Apr 18, 2012
#49 jsc...@chromium.org
(No comment was entered for this change.)
Status: Fixed
May 4, 2012
#50 scarybea...@gmail.com
This reward got old. If you like one option is that we up the reward to $1337 and donate it to charity.
May 9, 2012
#51 Michail....@gmail.com
Good idea, agreed.
Sep 4, 2012
#52 scarybea...@gmail.com
$1337 paid to Red Cross.
Labels: -reward-unpaid reward-decline
Sep 5, 2012
#53 Michail....@gmail.com
Thanks you guys.
Oct 13, 2012
#54 bugdro...@chromium.org
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.
Labels: Restrict-AddIssueComment-Commit
Mar 9, 2013
#55 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Area-WebKit -SecSeverity-Medium -WebKit-Core -Mstone-5 -Type-Security -SecImpacts-Stable Cr-Content M-5 Security-Impact-Stable Security-Severity-Medium Type-Bug-Security Cr-Content-Core
Mar 13, 2013
#56 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Mar 21, 2013
#57 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Security-Impact-Stable Security_Impact-Stable
Mar 21, 2013
#58 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Security-Severity-Medium Security_Severity-Medium
Apr 5, 2013
#59 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Cr-Content Cr-Blink
Sign in to add a comment

Powered by Google Project Hosting