Closed
Bug 344759
Opened 18 years ago
Closed 18 years ago
EIP hijack in FF 1.5.0.5 -- crash [@ 0x20202113] called by JS_SetPrivate
Categories
(Core :: Security, defect, P1)
Core
Security
Tracking
()
RESOLVED
FIXED
mozilla1.8.1beta2
People
(Reporter: darin.moz, Assigned: mrbkap)
References
Details
(Keywords: crash, fixed1.8.1, verified1.8.0.5, Whiteboard: [sg:critical?])
Crash Data
Attachments
(6 files, 3 obsolete files)
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
brendan
:
review+
dveditz
:
approval1.8.0.5+
mconnor
:
approval1.8.1+
|
Details | Diff | Splinter Review |
(deleted),
application/x-gzip
|
Details |
From H D Moore <hdm@metasploit.com>:
----------------------------------------------------------------------------
This bug is really annoying to track down, it seems like changing anything
in the script prevents the EIP hijack from working. Hope it works for
you, I can reliably reproduce here (Windows XP SP2 - FF 1.5.0.4).
-HD
(a18.470): Access violation - code c0000005 (!!! second chance !!!)
eax=20202020 ebx=00000000 ecx=20202020 edx=022e5308 esi=022e5310
edi=022e5308
eip=20202113 esp=0012d7c4 ebp=0012d7e8 iopl=0 nv up ei pl nz na pe
nc
cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000
efl=00000202
*** ERROR: Module load completed but symbols could not be loaded for C:
\WINDOWS\system32\xpsp2res.dll
xpsp2res+0x202113:
20202113 008000000606 add [eax+0x6060000],al
ds:0023:26262020=??
0:000> k
ChildEBP RetAddr
WARNING: Stack unwind information not available. Following frames may be
wrong.
0012d7e8 60082df1 xpsp2res+0x202113
0012d834 6009c92d js3250!JS_SetPrivate+0x47
0012d864 60082a79 js3250!js_GetSrcNoteOffset+0x400f
0012d8a4 6009c148 js3250!JS_InitClass+0xa3
0012d8e0 60081fd4 js3250!js_GetSrcNoteOffset+0x382a
0012d90c 60082321 js3250!JS_SetGlobalObject+0xc4
*** WARNING: Unable to verify checksum for C:\Program Files\Mozilla
Firefox\firefox.exe
*** ERROR: Symbol file could not be found. Defaulted to export symbols
for C:\Program Files\Mozilla Firefox\firefox.exe -
0012d928 0041021f js3250!JS_ResolveStandardClass+0x17c
0012d940 600addd2 firefox!NS_RegistryGetFactory+0x5ee0
0012d980 600ada90 js3250!js_LookupProperty+0x35b
0012d9a0 600ae29d js3250!js_LookupProperty+0x19
0012d9e0 004162e7 js3250!js_FindProperty+0x31b
0012da20 00416243 firefox!NS_RegistryGetFactory+0xbfa8
0012da4c 0040ce48 firefox!NS_RegistryGetFactory+0xbf04
0012dae0 0040cfb3 firefox!NS_RegistryGetFactory+0x2b09
0012db98 005364c5 firefox!NS_RegistryGetFactory+0x2c74
0012dbd8 00536e34 firefox!nsPrintOptions::GetNewPrintSettings+0x9baa
0012dbf4 005f298c firefox!nsPrintOptions::GetNewPrintSettings+0xa519
0012dc24 00732737 firefox!DeviceContextImpl::GetCanonicalPixelScale+0x9c53
0012dc4c 007247a5 firefox!nsPrintSettings::AddRef+0xb752
*** ERROR: Symbol file could not be found. Defaulted to export symbols
for C:\Program Files\Mozilla Firefox\xpcom_core.dll -
0012dc90 60311954 firefox!nsPrintSettings::GetPrintOptionsBits+0x14d0
----------------------------------------------------------------------------
See attached testcase.
I was able to reproduce this exploit using FF 1.5.0.4 and the pre-release version of FF 1.5.0.5 from today (July 14).
Reporter | ||
Comment 1•18 years ago
|
||
When successful, this webpage causes a find dialog to appear. In other cases, it has caused by browser to crash or hang.
Comment 3•18 years ago
|
||
Ok, I crash with a 2006-03-23 build, but don't crash with a 2006-03-24 trunk build. I tested this multiple times and on 2 different machines.
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-03-23+05&maxdate=2006-03-24+06&cvsroot=%2Fcvsroot
Note, I had to disable Java, because otherwise it would crash on the Java part in trunk (I think that's a known bug on trunk, at least there are some known Java crashers on trunk).
Comment 4•18 years ago
|
||
I managed to crash once with Firefox1.5RC3 (talkback ID: TB20767470W), but not anymore, afterwards.
Comment 5•18 years ago
|
||
I think I may have reduced it to this. However, I'm not sure (since I can't really reproduce the crash with the original testcase in FF1.5.0.5RC3).
But I am sure, that the first button is crashing also Firefox1.5.0.5RC3.
The second button is only crashing current trunk builds.
Comment 6•18 years ago
|
||
A typical talkback I get from testcase2 in Firefox1.5 is TB20978230Z:
kernel32.dll + 0x1eb33 (0x7c81eb33)
_CxxThrowException
_com_error::_com_error
_com_issue_error
CommonConstructor [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/XPCIDispatchExtension.cpp, line 90]
ActiveXConstructor
etc...
That's quite different from the original testcase, so that's why I'm not sure it's the same bug.
afaik we don't normally build w/ activex support. the build i have doesn't seem to:
--enable-application=browser --enable-update-channel=release --enable-optimize --disable-debug --disable-tests --enable-static --disable-shared --enable-official-branding --enable-svg --enable-canvas --enable-update-packaging
Comment 8•18 years ago
|
||
CCing the reporter, H D Moore.
Comment 9•18 years ago
|
||
This version doesn't hang Firefox and Mac OS X for 30 seconds while it loads ;) I haven't actually tested it, because I don't have ActiveX enabled.
Comment 10•18 years ago
|
||
Filed bug 344804 with a reduced testcase for a Java crash this turns up. I don't know if it's the same Java crash Martijn mentioned in comment 3.
Comment 11•18 years ago
|
||
I can't reproduce the crash in comment 0. With Firefox 1.5.0.4 on both Windows XP and Mac, as well as with yesterday's 1.5.0.x nightly on Mac, I get a few alert-like things and a Find dialog, and my browser window is resized, but Firefox does not crash.
Comment 12•18 years ago
|
||
Crashing [@ 0x20202113] in the JavaScript engine probably means this is a GC hazard bug. GC hazard bugs tend to be hard to reduce :( I think the fastest way to get this fixed on 1.5.0.x is to determine what trunk patch fixed it.
Martijn, is the "fixed on trunk" period you gave in comment 3 for the GC-hazard-looking crash in comment 0, or is it for the ActiveX crash in comment 5? I'm hoping it's the latter, since there weren't any changes to the JavaScript engine or XPConnect on that day that look like GC hazard fixes.
Keywords: crash
Summary: EIP hijack in FF 1.5.0.5 → EIP hijack in FF 1.5.0.5 -- crash [@ 0x20202113] called by JS_SetPrivate
Whiteboard: [sg:critical?]
Version: Trunk → 1.8 Branch
Updated•18 years ago
|
Flags: blocking1.9a1+
Flags: blocking1.8.1?
Flags: blocking1.8.0.5+
Comment 13•18 years ago
|
||
I couldn't reproduce the original testcase either until I created a new profile, then *boom*. So if you're having trouble reproducing, try that. There's probably some common power-user pref setting a lot of us have that's interfering.
I died in JS_SetPrivate a lot (different call stacks, though): TB20989667, TB20989639, TB20989449
Once under ActiveXConstructor (TB20987029), similar to Martijn's comment 6.
Have not been able to reproduce with a debug build. I get a single "NATIVE:window" line added each time I press the button, and on the console "An error occurred updating the cmd_copy command". Otherwise nothing.
Comment 14•18 years ago
|
||
(In reply to comment #12)
> Martijn, is the "fixed on trunk" period you gave in comment 3 for the
> GC-hazard-looking crash in comment 0, or is it for the ActiveX crash in comment
> 5?
It's not for the ActiveX crash, I just tested that. Not sure what it is.
A typical talkback ID, I get is: TB20990092X
js3250.dll + 0x44f0e (0x60104f0e)
firefox.exe + 0x48f9c4 (0x0088f9c4)
xpcom_core.dll + 0x354e9 (0x604954e9)
firefox.exe + 0x1de24 (0x0041de24)
Comment 15•18 years ago
|
||
> I'm hoping it's the latter, since there weren't any changes to the
> JavaScript engine or XPConnect on that day that look like GC hazard fixes.
The bonsai link doesn't go back far enough, just misses Igor's 3:50 check-in for bug 330692 ("GC_MARK_DEBUG: using GC_MARK, not JS_MarkGCThing internally"). Let me see if applying that does the trick (may need to drag bug 324278 along, too).
Comment 16•18 years ago
|
||
Igor's patches don't apply cleanly to my tree (not unexpected). I'm too sleepy to fix it up tonight and travelling most of tomorrow, can anyone else look at it? If not I'll try again Monday.
Comment 17•18 years ago
|
||
Blake - can you take a look?
Comment 18•18 years ago
|
||
Ok, the "fixed on trunk" period I gave in comment 3 appears to come from the fix from bug 330900. I've checked that by building a 2006-03-23 build and manually applying the patch from that bug.
That means that the regression range in comment 3 is not relevant for this bug :(
Updated•18 years ago
|
Flags: blocking1.8.1? → blocking1.8.1+
Comment 19•18 years ago
|
||
Do we have a fixed-on-trunk window for testcase 2?
Updated•18 years ago
|
Flags: blocking1.8.1+ → blocking1.8.1?
Comment 20•18 years ago
|
||
Comment on attachment 229352 [details]
testcase2
(In reply to comment #19)
> Do we have a fixed-on-trunk window for testcase 2?
Sorry, testcase 2 is most likely not related to this bug (I thought it was, first).
Testcase2 is not fixed on trunk, I'll file a new bug on it.
Attachment #229352 -
Attachment is obsolete: true
Updated•18 years ago
|
Attachment #229367 -
Attachment is obsolete: true
Comment 21•18 years ago
|
||
Sorry, I somehow reset the blocking1.8.1+ back to blocking1.8.1?
I filed bug 344957 and bug 344960 for testcase2, which isn't related to this bug.
Updated•18 years ago
|
Flags: blocking1.8.1? → blocking1.8.1+
Comment 22•18 years ago
|
||
(In reply to comment #18)
> Ok, the "fixed on trunk" period I gave in comment 3 appears to come from the
> fix from bug 330900. I've checked that by building a 2006-03-23 build and
> manually applying the patch from that bug.
> That means that the regression range in comment 3 is not relevant for this bug
Why do you say that? The stacks from bug 330900 were different from this bug, and in addition the current 1.8/1.8.0 branches also have the 330900 fix but still crash on the first testcase here. I've not been able to reproduce this crash with any debug build.
I confirm your regression range in comment 3 (2006-03-23-05 broken, 2006-03-24-05 fixed) but my money's still on Igor's GC patch.
Comment 23•18 years ago
|
||
(In reply to comment #13)
> Have not been able to reproduce with a debug build. I get a single
I saw the same problem. mrbkap explained it to me: the string representations have some more information in them, which includes the word "native". So this change makes it run in a DEBUG build:
@@ -26,7 +26,7 @@
function msftestIsNative(obj) {
var txt = obj + '';
- if (txt.indexOf('native') != -1) {
+ if (txt.indexOf('[native') != -1) {
return true;
}
return false;
Comment 24•18 years ago
|
||
(In reply to comment #22)
> Why do you say that? The stacks from bug 330900 were different from this bug,
> and in addition the current 1.8/1.8.0 branches also have the 330900 fix but
> still crash on the first testcase here. I've not been able to reproduce this
> crash with any debug build.
I don't crash with 1.8/1.8.0 branches (only very sporadically). When I add && e.indexOf('CRMFRequest') == -1 to the testcase on the relevant spots, then I don't crash anymore with a 2006-03-23 trunk build.
Comment 25•18 years ago
|
||
Comment 26•18 years ago
|
||
Comment 27•18 years ago
|
||
(In reply to comment #26)
> Created an attachment (id=229642) [edit]
> reduced testcase invoke window.find on buf
works in 1.8.0.5, 1.8, 1.9 winxp
Comment 28•18 years ago
|
||
(In reply to comment #13)
> I couldn't reproduce the original testcase either until I created a new
> profile, then *boom*. So if you're having trouble reproducing, try that.
> There's probably some common power-user pref setting a lot of us have that's
> interfering.
>
> I died in JS_SetPrivate a lot (different call stacks, though): TB20989667,
> TB20989639, TB20989449
The stack traces shows
JS_SetPrivate [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 2164]
js_DefineFunction [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsfun.c, line 2125]
JS_InitClass [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 2045]
but js_DefineFunction does not call JS_SetPrivate directly, so the stack traces are not complete. Why it is so?
Comment 29•18 years ago
|
||
FWIW, I ran mrbkap's variant of the original testcase under WAY_TOO_MUCH_GC on my 1.8.0 branch Windows build, and it didn't crash.
Comment 30•18 years ago
|
||
> this change makes it run in a DEBUG build:
Thank you (and mrbkap) for that!
> > reduced testcase invoke window.find on buf
> works in 1.8.0.5, 1.8, 1.9 winxp
Define "works"? I don't crash using that testcase on either trunk or 1.5.0.4
Assignee | ||
Comment 31•18 years ago
|
||
This is the only object that could possibly be the cause of the crash based on the stacks that we have seen. The basic principal here is that we cannot trust newborn roots!
Comment 32•18 years ago
|
||
Attachment #229765 -
Flags: review?
Attachment #229765 -
Flags: approval1.8.0.5?
Comment 33•18 years ago
|
||
This fixes the problem in my 1.8.0.5 builds (debug and optimized). Reproducing the original crash in the debug build was iffy, but optimized was a consistent crasher (despite the occassional need to create new profiles, or delete the prefs.js file) and it's fixed now.
Comment 34•18 years ago
|
||
Comment on attachment 229759 [details] [diff] [review]
Potential fix
The comment might say "any potential GC callback."
r=me, looks good.
/be
Attachment #229759 -
Flags: review?(brendan) → review+
Comment 35•18 years ago
|
||
Comment on attachment 229765 [details] [diff] [review]
merged to 1.8 branches
r=me with same comment suggestion as for the trunk/1.8-branch.
/be
Attachment #229765 -
Flags: review? → review+
Assignee | ||
Comment 36•18 years ago
|
||
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment 37•18 years ago
|
||
Comment on attachment 229765 [details] [diff] [review]
merged to 1.8 branches
approved for 1.8.0 branch, a=dveditz
Attachment #229765 -
Flags: approval1.8.1?
Attachment #229765 -
Flags: approval1.8.0.5?
Attachment #229765 -
Flags: approval1.8.0.5+
Comment 39•18 years ago
|
||
(In reply to comment #31)
> The basic principal here is that we cannot trust newborn roots!
But that's a bug in itself, and a regression at some point that we did not catch. We need to fix it, too, and centrally. Short of fixing bug 313437, we should not allow JS_ClearNewbornRoots to do anything when called from the GC allocator.
Would you please file that bug?
/be
Comment 40•18 years ago
|
||
(In reply to comment #39)
> (In reply to comment #31)
> > The basic principal here is that we cannot trust newborn roots!
>
> But that's a bug in itself, and a regression at some point that we did not
> catch. We need to fix it, too, and centrally. Short of fixing bug 313437, we
> should not allow JS_ClearNewbornRoots to do anything when called from the GC
> allocator.
>
> Would you please file that bug?
This JS_ClearNewbornRoots hazard arose due to the null-script branch callback case, added for bug 203278. But it exists also in theory due to the JSGC_END mode GC-callback, used by Venkman and other modules.
/be
Updated•18 years ago
|
Attachment #229765 -
Flags: approval1.8.1? → approval1.8.1+
Comment 41•18 years ago
|
||
Comment 42•18 years ago
|
||
v.fixed on 1.8.0 branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.5) Gecko/20060719 Firefox/1.5.0.5, no crash with testcase. I was able to reliably crash with rc3 with the following steps:
1. launch with profile manager flag (-P)
2. create new profile
3. go to this bug, login to bugzilla
4. open testcase and click button
5. boom!
I crashed about 5-6 times in a row with the previous build (rc3), but have not been able to reproduce with latest rc4 build after about 15 tries (same basic steps above with a few variations thrown in).
Keywords: fixed1.8.0.5 → verified1.8.0.5
Comment 43•18 years ago
|
||
In Firefox 1.0.8 this particular testcase crashes on bug 330900 instead. Once that's fixed on that branch this should be retested. It may or may not crash due to the timing issues involved.
Flags: blocking1.7.14?
Flags: blocking-aviary1.0.9?
Comment 44•18 years ago
|
||
Not needed on the aviary or moz1.7 branches since bug 203278 was not fixed until after those branches.
Flags: blocking1.7.14?
Flags: blocking1.7.14-
Flags: blocking-aviary1.0.9?
Flags: blocking-aviary1.0.9-
Assignee | ||
Comment 45•18 years ago
|
||
Fix checked into the 1.8 branch.
Keywords: fixed1.8.1
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.8.1beta2
Version: 1.8 Branch → Trunk
Comment 46•18 years ago
|
||
Unfortunately, this bug does exist for 1.7.x and 1.0.x.
Dan, are you sure this was introduced with fix for bug #203278 ? I have not backported that, nevertheless *all* testcases do apply, so probably it was a different fix that triggered this. Any idea?
Comment 47•18 years ago
|
||
Did anyone verify this on Linux? Our QA seems to suggest that this is still not fixed on Linux... Will try to get more details.
Comment 48•18 years ago
|
||
(To clarify, they say that 1.5.0.5 still exhibits this bug on Linux...) Will update with details as I get them.
Comment 49•18 years ago
|
||
(In reply to comment #48)
> (To clarify, they say that 1.5.0.5 still exhibits this bug on Linux...) Will
> update with details as I get them.
>
At least backporting the branches fix attached did not help for 1.0.x ... so probably its really not fixed for linux.
Assignee | ||
Comment 50•18 years ago
|
||
(In reply to comment #49)
> At least backporting the branches fix attached did not help for 1.0.x ... so
> probably its really not fixed for linux.
Do you have backtraces of any crashes you see? The testcase for this bug is *extremely* generic, and it's possible that fixing this bug is exposing another bug.
Comment 51•18 years ago
|
||
So I went and had a look at the testing boxes, and it appears to not be a crash anymore. It's simply an exit(1) which is an improvement, but we should still get it to work properly. Anyway, here's the strace output.
Assignee | ||
Comment 52•18 years ago
|
||
(In reply to comment #51)
> So I went and had a look at the testing boxes, and it appears to not be a crash
> anymore. It's simply an exit(1) which is an improvement, but we should still
> get it to work properly. Anyway, here's the strace output.
If I'm reading the strace correctly, it seems that X becomes unhappy with us for whatever reason (the end of the log is opening XErrorDB and writing out something about 'Gecko') and kills us. I think that deserves its own bug.
Comment 53•18 years ago
|
||
This is a backtrace for 1.0.8 without the backported patch.
Further there was no patch for bug 203278 applied - see comment #44
In response to comment 43 I used the patch from bug 330900 ... but it makes no difference at all. The backtrace looks similar.
Assignee | ||
Comment 54•18 years ago
|
||
(In reply to comment #53)
> Further there was no patch for bug 203278 applied - see comment #44
> In response to comment 43 I used the patch from bug 330900 ... but it makes no
> difference at all. The backtrace looks similar.
The backtrace looks similar to what? That looks like a very different problem, presumably fixed by some other bug on the trunk (and probably on the 1.8 branch). This particular crash is actually controlled, it will assert (abort in debug builds, and return false, silently halting the script's execution).
Comment 55•18 years ago
|
||
(In reply to comment #54)
> (In reply to comment #53)
> > Further there was no patch for bug 203278 applied - see comment #44
> > In response to comment 43 I used the patch from bug 330900 ... but it makes no
> > difference at all. The backtrace looks similar.
>
> The backtrace looks similar to what?
I meant similar to the backtrace from a build without the patch from bug 330900.
> branch). This particular crash is actually controlled, it will assert (abort in
> debug builds, and return false, silently halting the script's execution).
>
Fine, that's good enough for me I guess.
Updated•18 years ago
|
Attachment #231138 -
Attachment is obsolete: true
Updated•17 years ago
|
Group: security
Updated•17 years ago
|
Flags: in-testsuite?
Updated•13 years ago
|
Crash Signature: [@ 0x20202113]
You need to log in
before you can comment on or make changes to this bug.
Description
•