Closed
Bug 439276
Opened 17 years ago
Closed 14 years ago
Silent script termination when calling atob() with illegal input in JS component
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: ma1, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
patch
|
shaver
:
review+
shaver
:
superreview+
|
Details | Diff | Splinter Review |
Calling atob() with an illegal argument from a JavaScript component leads to a weird "silent" code interruption.
While the same call atob() from a DOM Window throws an exception, if the context is a JS component the code "exits" with *no exception thrown* and *no crash*. It apparently just stop executing, making error handling or predictable execution impossible.
Should further analysis show this can happen with other JS APIs, JavaScript components should be considered very unreliable all at once.
This may also have security implications.
Notice that this happens on Gecko 1.9 and above only. Firefox 2 behaves as expected, throwing an exception no matter if the global object is a DOM Window or not.
Step to reproduce:
1. Install the AdBlock Plus extension, https://addons.mozilla.org/en-US/firefox/addon/1865
2. Open Tools|Error Console
3. Evaluate the following code line:
try { atob("/"); } catch(e) { alert(e) } try { alert(Components.classes["@mozilla.org/adblockplus;1"] .createInstance().wrappedJSObject.__parent__.eval('atob("/")')) } catch(e) { alert(e) }; alert("Done!");
Expected result:
You should see 3 alerts, first 2 with error reports and the last saying "Done!"
Actual result:
Only the first error alert is displayed. Code aborts abruptly, with no error message, but the browser keeps running.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008060607 Minefield/3.0pre
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1a1pre) Gecko/2008061011 Minefield/3.1a1pre
Reporter | ||
Comment 1•17 years ago
|
||
Confirmed on
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1a1pre) Gecko/2008061402 Minefield/3.1a1pre
and
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008061406 Minefield/3.0pre
Updated•17 years ago
|
Flags: blocking1.9?
I don't know why there would be security implications, can you sketch them out? What I see here is a problem with error reporting, though your test case doesn't show it being called from a JS component but rather being called from chrome and operating on a wrappedJSObject in an unusual way. (Via indirect eval, to boot.)
If you have a JS component that simply called atob('/') at load time, f.e., do you not see an exception? A smaller test case is necessary to confirm your hypothesis here, I think, and I wouldn't be surprised if the bug is really "indirect eval called on non-window object via wrapper doesn't call error reporter correctly", rather than anything to do with JS components and atob. Nonetheless, moving to XPConnect where a bug with JS component error reporting would belong.
Severity: major → normal
Component: DOM → XPConnect
Flags: blocking1.9? → blocking1.9-
QA Contact: general → xpconnect
Summary: Weird abort (no crash, no exception) calling atob() from JS component → No exception reported when calling atob() with illegal argument from JS component
Reporter | ||
Comment 3•17 years ago
|
||
Mike, this is a bit more worrisome than "error not being reported", IMHO.
If you looked carefully at my test case, you'd notice that you don't get the final "Done!" alert either.
The control flow gets disrupted, our code just stops and the native caller goes on as nothing happened: a whole execution path vanished abruptly.
A crash would have been safer at this point.
Security implications (just an example):
SomeContentPolicy = {
shouldLoad: function(contentType, location) {
try {
callToSomeBuggyAPILikeAtob();
// this bug will make us always exit abruptly here,
// turning this policy in default allow
if(checkSecurityWhiteList(location)) return nsIContentPolicy.ACCEPT;
} catch(ex) {
// safety net, we wanted to always reject if something went wrong...
}
// .. but we never reach here
return nsIContentPolicy.REJECT_SERVER;
}
}
-- Test case to show how content policies could get broken --
1) Install the NoScript extension
2) Enter the following on the error console:
top.opener.noscriptOverlay.ns.shouldLoad = function(ctype, location, origin) { if (ctype != 3 || 'www.mozilla.com' != origin.host) return 1; try { this.__parent__.eval('atob("/")') } catch(e) {} finally {return 3}}; open("http://www.mozilla.com");
Expected result:
The MoCo home page should be shown with no image inside
Actual result:
The MoCo home page is shown with all its images
-- Test case to demonstrate this is not an eval weirdness --
1) Open the $MOZILLA_HOME/components/aboutRobots.js
2) Insert the following code at the beginning of the AboutRobots.newChannel() method (line 55):
try { atob("/") } catch(e) {}
3) Launch Firefox and try to open "about:robots"
Expected result:
An error page featuring a cute robot
Actual result:
Nothing happens
-- Notice --
My main concern about this bug, if it's not clear enough yet, is that a finally {} statement should be always guaranteed to run. Changing back description and severity to reflect this.
Also, I would be at least a bit relieved in knowing it's atob() only (I already implemented work-arounds in my code), but until somebody more knowledgeable than me can look into this issue, shouldn't we assume other APIs may be affected as well?
Severity: normal → major
Summary: No exception reported when calling atob() with illegal argument from JS component → Weird abort (neither catch{} nor finally{} run, but no crash either) after atob() errors in JS component
Updated•17 years ago
|
Assignee: nobody → crowder
Yeah, it's a bug in atob (probably mine from long ago): if PL_Base64Decode returns failure, we silently abort the script. :( Thanks, Giorgio, for the clarification and narrower test case; made it much easier to figure out by analysis.
Crowder: thanks for volunteering to take this. We should probably do what the content case does, since
javascript:try { atob("/"); } catch (e) { alert(e); }
dtrt. btoa has the same bug, but it's harder to trip since it's hard to find a string that's not legal to _encode_ as base 64. :)
Severity: major → normal
Summary: Weird abort (neither catch{} nor finally{} run, but no crash either) after atob() errors in JS component → Silent script termination when calling atob() with illegal input in JS component
(finally also doesn't run in the case of OOM, AIUI -- FWIW, just FYI. :) )
Comment 6•17 years ago
|
||
Attachment #325347 -
Flags: superreview?(jst)
Attachment #325347 -
Flags: review?(shaver)
Comment 7•17 years ago
|
||
Ideas on implementing an automated test for this are much appreciated.
Comment on attachment 325347 [details] [diff] [review]
JS_ReportError makes this a catchable exn
r+sr=shaver
Attachment #325347 -
Flags: superreview?(jst)
Attachment #325347 -
Flags: superreview+
Attachment #325347 -
Flags: review?(shaver)
Attachment #325347 -
Flags: review+
Comment 9•14 years ago
|
||
I've obviously been a terrible owner for this bug.
Assignee: crowderbt → nobody
Comment 10•14 years ago
|
||
Pretty sure this has been fixed as part of bug 608170. At least xpcshell tests that would just silently cut out and fail with Firefox 3.6 now produce nice exceptions for me when dealing with improperly padded base64.
You need to log in
before you can comment on or make changes to this bug.
Description
•