308284 - How to easily fix Firefox extension crashes

Status: RESOLVED INVALID

(Firefox :: General, defect)

Description

[transient300]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6

Firefox generates an access violation at address 0, i.e. "0xC0000005: Access
violation reading location 0x00000000," when accessing some pages containing
flash animations.  This is almost certainly an attempt to dereference a null
pointer.  This occurs if you have version 1.2.9 of the Flashblock extension.  It
does NOT happen with the latest version of Flashblock or without Flashblock.

Now, of course, there must be a bug in the Flashblock extension itself, which
has been fixed.  This bug report is not about Flashblock.  This bug report is
about the ability of an extension to cause Firefox to crash.

Flashblock is a very popular extenstion, second only to AdBlock.  This bug could
also happen with any extension that contains a flaw.  This is extremely
important as extensions are a leading reason why people prefer Firefox over IE,
and without such protection, an extension can easily bring Firefox down.

The most desirable behavior would be to have an insolation layer to protect
Firefox from ill-behaved extensions.  

Luckily, there is a simple way to protect Firefox from almost everything that a
wayward extension would likely do.  This strategy would work in Java, C#, or
C++.  Since I suspect that Firefox is written in C++, I'll give the example in
that language.

The essence of the strategy is to restrict extensions from creating their own
threads except via a runThread function that creates the thread and invokes a
"run" function or method for the extension.  Now whenever the Firefox
application hands control over to an extension (by calling some function of the
extension) it should:
1. Do so only in a worker thread created either by Firefox internally or by
Firefox within the runThread function.
2. Surround the call the extension's function with a try-catch block that
catches all exceptions.

The following C++ program illustrates how to do this.  Without the try-catch
block, the program will crash generating the "0xC0000005: Access violation
reading location 0x00000000" in Windows or a core dump in Unix (for a Unix
equivalent program).  With the try-catch block, nothing bad happens.

--- Start of program ---
#include "stdafx.h"

int _tmain(int argc, _TCHAR* argv[])
{
    long *x = 0;
    
    try
    {
        printf("The answer is %ld\n", *x);
    }
    catch (...)
    {
        printf("Not so fast\n");
    }
    
    printf("Done\n");
    
    return 0;
}
--- End of program ---

This technique works for divisions by zeros, accessing non-null addresses
outside of the program's memory area, illegal op codes, and most other things
that go wrong in other people's code.  I have used this technique quite
successfully to protect myself from other programmers in both Java and C++
environments.  It protects me from 100% of the "real world" screw-ups and only
fails if I write a function intended to *make it fail* by doing something
deliberate like overwriting the caller's memory space.  But stuff like that does
not happen accidentally.  So this will protect you against the mistakes that do
happen in real production systems.  It's what I call "defensive programming."

If you have any questions, you can reach me at the email address: barbalace at
clearthought.info.  (Don't let bots learn the address).


Reproducible: Always

Steps to Reproduce:
1. Install version 1.2.9 of the Flashblock extension.  You can find it at
https://addons.mozilla.org/extensions/showlist.php?application=firefox&category=Web%20Annoyances&numpg=10&pageid=2
2. Go to the page
http://www.google.com/url?sa=t&ct=res&cd=29&url=http%3A//www.world-of-newave.info/computers/2004-november-16.htm&ei=5V4mQ6_uEcru4AGfy4DsDQ
3. Wait for the crash

Actual Results:  
Windows displayed the generic application caused and exception message with an
option to send Microsoft the information.  Firefox displayed its feedback dialog
box for crashes.

Expected Results:  
The Firefox application should have isolated itself from the access violation by
invoking extension code within a try-catch block and making sure extensions
cannot directly spawn threads so that Firefox can ensure that those threads also
only call extension code within a try-catch block.


This could also be considered a security problem in that it is possible that
someone might deliberately write an extension that periodically causes Firefox
to crash for no plausible reason.  Something like

    if today is Monday, Thursday, or Friday
        sleep for ten minutes then
            crash Firefox unexpectedly

Such an extension might not be readibly identifiable as the cause of the
crashes.  So don't tell Microsoft ;)

Comment 1

[Daniel Veditz [:dveditz]]

Clearing confidential flag: it's not protecting a specific security exploit and
this deserves broader discussion

Comment 2

[timeless]

we can't do this. i've tried you end up with the spidermonkey vm killing itself 
when it gets its gc state out of sync.

thank you for your suggestions.

Comment 3

[Robert Strong (they/them - no direct email)]

The extension manager manages an extension's install, upgrade, etc. but it is
not involved with how extensions interact with the app in relation to this bug
report. Changing component to General

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

fwiw, using catch(...) to catch stuff like null dereferences will _only_ work on msvc, no other compiler does this.

Comment 5

[Wayne Mery (:wsmwk)]

timeless comment #2:
> we can't do this. i've tried you end up with the spidermonkey vm killing itself 
> when it gets its gc state out of sync.

so this is a non-starter ?

Comment 6

[timeless]

yes, it's a nonstarter.

i'm going to resolve this as invalid, because while the reporter things it's clever, it can't work. i don't want people to accuse me of "wontfix"ing something that could "easily fix firefox extension crashes".