494595 - potential overflow in EvaluateAdminConfigScript

Status: NEW

(Core :: Preferences: Backend, defect)

Description

[Brian Hackett [Laid off!]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.10) Gecko/2009042316 Firefox/3.0.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.10) Gecko/2009042316 Firefox/3.0.10

Hi, it looks like there is a read buffer overflow in EvaluateAdminConfigScript (extensions/pref/autoconfig/src/nsJSConfigTriggers.cpp) which can lead to an integer overflow and a buffer being marked as UINT_MAX bytes long.

I doubt this is exploitable (it looks like this is only called on streams read from a local file), but I'm marking it as a potential security problem to be careful.

The relevant code is pasted below.

BEGIN_CODE

nsresult EvaluateAdminConfigScript(const char *js_buffer, size_t length,
                                   const char *filename, PRBool bGlobalContext, 
                                   PRBool bCallbacks, PRBool skipFirstLine)
{
    JSBool ok;
    jsval result;

    if (skipFirstLine) {
        /* In order to protect the privacy of the JavaScript preferences file 
         * from loading by the browser, we make the first line unparseable
         * by JavaScript. We must skip that line here before executing 
         * the JavaScript code.
         */
        unsigned int i = 0;
        while (i < length) {
            char c = js_buffer[i++];
            if (c == '\r') {
                if (js_buffer[i] == '\n')
                    i++;
                break;
            }
            if (c == '\n')
                break;
        }

        length -= i;
        js_buffer += i;
    }

    ...

    JS_BeginRequest(autoconfig_cx);
    ok = JS_EvaluateScript(autoconfig_cx, autoconfig_glob,
                           js_buffer, length, filename, 0, &result);
    ...
}

END_CODE

If the last character in js_buffer is '\r', the test '(js_buffer[i] == '\n')' overflows js_buffer by one byte.  If this test happens to succeed (the garbage character after js_buffer is '\n'), i will be 'length + 1' after exiting the loop and the subtraction 'length -= i' will integer overflow to UINT_MAX.  This value of length is then passed to JS_EvaluateScript.


Reproducible: Always

Comment 1

[Daniel Veditz [:dveditz]]

This would be a pretty far-fetched attack: if the attacker could modify this config file they could more easily hack Firefox by changing/replacing one of the code libraries with a variant that does some dastardly deed. But you're right there's a bug here.

Comment 2

[Josh Aas]

Opening up per comment 1, which indicates that this is not a significant security issue. This bug is more likely to get fixed if this is open.