876568 - Random segfault in imgCacheValidator::OnStartRequest (mRequest is null when channelURI is non-null)

Status: RESOLVED DUPLICATE of bug 1148640

(Firefox :: Security, defect)

Description

[Superfluous]

* What did you do?

When I use the Tor Browser Bundle (2.3.25-8, based on Portable Apps's Firefox ESR 17.0.6, Vidalia 0.2.21, and Tor 0.2.3.25) on Windows, it crashes ("tbb-browser.exe has stopped working") after about a whole day of use on a particular discussion site with many other tabs open (maybe 15-20). The regular Firefox stable (now on version 22) almost never crashes for me, with typically double the number of tabs open, plugins, etc.

The second time it crashed, OllyDbg was unable to attach to the crashed process, so I restarted TBB and attached OllyDbg to tbb-browser.exe and waited. After about 6-8 hours, it finally crashed:

Access violation when reading [0x00000028] in CPU - main thread at xul+0xE271B (xul.66B6271B) - Application was unable to process exception.

* What happened?

When this happened, ollydbg.exe and tbb-browser.exe together had a total of 260MB allocated, which is next to nothing (I have another 1GB of free memory).

The crash occurs here:

CPU Disasm
Address   Hex dump          Command                                  Comments
66B62704  |.  8B0F          MOV ECX,DWORD PTR DS:[EDI]
66B62706  |.  50            PUSH EAX
66B62707  |.  57            PUSH EDI
66B62708  |.  FF51 3C       CALL DWORD PTR DS:[ECX+3C]
66B6270B  |.  8B45 08       MOV EAX,DWORD PTR SS:[EBP+8]
66B6270E  |.  3BC3          CMP EAX,EBX
66B62710  |.- 74 13         JE SHORT 66B62725
66B62712  |.  8B08          MOV ECX,DWORD PTR DS:[EAX]
66B62714  |.  8D55 13       LEA EDX,[EBP+13]
66B62717  |.  52            PUSH EDX
66B62718  |.  8B56 24       MOV EDX,DWORD PTR DS:[ESI+24]
66B6271B  |.  FF72 28       PUSH DWORD PTR DS:[EDX+28]               ; Crash
66B6271E  |.  50            PUSH EAX
66B6271F  |.  FF51 58       CALL DWORD PTR DS:[ECX+58]
66B62722  |.  8B45 08       MOV EAX,DWORD PTR SS:[EBP+8]
66B62725  |>  385D 12       CMP BYTE PTR SS:[EBP+12],BL


The registers are:

Registers
EAX 0F2FCAB0
ECX 673BDCB8 xul.673BDCB8
EDX 00000000
EBX 00000000
ESP 0042D1C4
EBP 0042D214
ESI 1D7606E0
EDI 1741DBB0
EIP 66B6271B xul.66B6271B

C 0  ES 002B 32bit 0(FFFFFFFF)
P 0  CS 0023 32bit 0(FFFFFFFF)
A 0  SS 002B 32bit 0(FFFFFFFF)
Z 0  DS 002B 32bit 0(FFFFFFFF)
S 0  FS 0053 32bit FFFDD000(FFF)
T 0  GS 002B 32bit 0(FFFFFFFF)
D 0
O 0  LastErr 00000000 ERROR_SUCCESS
EFL 00010202 (NO,NB,NE,A,NS,PO,GE,G)

ST0 empty 0.0
ST1 empty 529.00000000000000000
ST2 empty 8192.0000000000000000
ST3 empty 0.0
ST4 empty 1058.0000000000000000
ST5 empty -0.0
ST6 empty 2147746065.0000000000
ST7 empty 2147746065.0000000000
               3 2 1 0      E S P U O Z D I
FST 4020  Cond 1 0 0 0  Err 0 0 1 0 0 0 0 0 (EQ)
FCW 027F  Prec NEAR,53  Mask    1 1 1 1 1 1
Last cmnd 0000:00000000

XMM0 00000000 00000000 00000000 00000000
XMM1 00000000 00000000 00000000 00000000
XMM2 00000000 00000000 00000000 00000000
XMM3 00000000 00000000 00000000 00000000
XMM4 00000000 00000000 00000000 00000000
XMM5 00000000 00000000 00000000 00000000
XMM6 226C6D74 68782F39 3939312F 67726F2E
XMM7 68206174 656D3C0A 3E646165 683C0A3E
                                P U O Z D I
MXCSR 00001FA1  FZ 0 DZ 0  Err  1 0 0 0 0 1
                Rnd NEAR   Mask 1 1 1 1 1 1


As this was with the precompiled TBB binary which lacks debugging symbols, a string referenced further down in the function helped me identify that the crash occurred in imgCacheValidator::OnStartRequest(), implemented in mozilla-esr17/image/src/imgLoader.cpp.

The disassembly given above corresponds to the following code (line 2086 of imgLoader.cpp):

    channel->GetURI(getter_AddRefs(channelURI));
    if (channelURI)
      channelURI->Equals(mRequest->mCurrentURI, &sameURI);

For whatever reason, mRequest was null at this instance when channelURI was non-null. This error is, as far as everything seems, perfectly recoverable (TBB will resume working) if you return if mRequest was null. (Similar segfaults will occur at xul+0xE2CC3 (xul.66B62CC3), xul+0xE2818 (xul.66B62818), and so in which is why you must actually leave imgCacheValidator::OnStartRequest.)

I'm attaching a partial stack dump and the memory of the relevant objects. As to whether the bug was introduced by Mozilla, Portable Apps, or Tor, I'm not certain, so I'm just reporting it to all three.

* What should have happened?

The program should not have crashed. Either a guard should be added to check for a null mRequest (and return if this is the case), an exception handler should be added (these are already all over the place), or an evaluation could be conducted to find why mRequest was null in the first place.

Comment 1

[Superfluous]

Attachment: tor-browser-2.3.25-8-dump.txt

Comment 2

[Superfluous]

Attachment: Adds if (!mRequest) return NULL; as imgCacheValidator::OnStopRequest() already does with mDestListener

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 3

[bhavana bajaj [:bajaj]]

This bug does not meet the ESR landing criteria, where the bug has to be a sec-high/crit issue, major stability issue (landing/merged on other branches like aurora,beta) or fixes for regressions specific to the ESR hence an a- here.

Comment 4

[Georg Koppen]

Does not seem to be true(In reply to bhavana bajaj [:bajaj] from comment #3)
> This bug does not meet the ESR landing criteria, where the bug has to be a

Does not seem to be true, see: bug 1148640. Marking this as a duplicate of it.