Closed Bug 439276 Opened 12 years ago Closed 9 years ago

Silent script termination when calling atob() with illegal input in JS component

Categories

(Core :: XPConnect, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: mao, Unassigned)

References

Details

Attachments

(1 file)

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
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
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
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
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. :) )
Attachment #325347 - Flags: superreview?(jst)
Attachment #325347 - Flags: review?(shaver)
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+
I've obviously been a terrible owner for this bug.
Assignee: crowderbt → nobody
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.
Status: NEW → RESOLVED
Closed: 9 years ago
Depends on: 608170
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.