Use-after-free error with evalcx() under gczeal

UNCONFIRMED
Unassigned

Status

()

Core
JavaScript Engine
UNCONFIRMED
7 years ago
3 years ago

People

(Reporter: Alex Miller, Unassigned)

Tracking

({testcase, valgrind})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [debug-only])

Attachments

(2 attachments, 2 obsolete attachments)

33 bytes, application/javascript
Details
3.64 KB, text/plain
Details
(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.12) Gecko/20101027 Ubuntu/10.10 (maverick) Firefox/3.6.12
Build Identifier: 

When modifying the testcase of https://bugzilla.mozilla.org/show_bug.cgi?id=596103#c10 I found that changing the first gc() call to evalcx(gc()) causes a use-after-free error to be reported in valgrind.

Reproducible: Always

Steps to Reproduce:
1.Attach valgrind to ./js t.j+s
Actual Results:  
No crash or assertion, but valgrind reports:
Invalid read of size 4
Address 0x44627a8 is 64 bytes inside a block of size 465 free'd (Always the same address too)

Expected Results:  
The program should execute without valgrind identifying any serious errors.
(Reporter)

Comment 1

7 years ago
Created attachment 497015 [details]
Testcase
(Reporter)

Updated

7 years ago
Keywords: testcase, valgrind
(Reporter)

Comment 2

7 years ago
(In reply to comment #0)
Steps to Reproduce:
1.Attach valgrind to ./js t.js

Stupid typos...
This seems to be a missing call of JS_GetScriptObject again.
The conservative stack scanner only marks a JSScript with the scriptObject.
The code in js.cpp definitely misses this:

        script = JS_CompileFileHandle(cx, obj, filename, file);
        JS_SetOptions(cx, oldopts);
        if (script) {
            if (!compileOnly) {
                (void)JS_ExecuteScript(cx, obj, script, NULL);
                int64 t2 = PRMJ_Now() - t1;
                if (printTiming)
                    printf("runtime = %.3f ms\n", double(t2) / PRMJ_USEC_PER_MSEC);
            }
            JS_DestroyScript(cx, script);

I don't think this is a security issue in this case.
(Reporter)

Comment 4

7 years ago
Created attachment 497027 [details]
What valgrind has to say about it
Yeah that comes from the Assert in JS_DestroyScript:
    
JS_ASSERT(script->u.object);

The script was deleted during the last GC.
This assert is bogus because the conservative stack scanner takes care of the lifetime and there is no guarantee that the script is still alive.
(Reporter)

Comment 6

7 years ago
Created attachment 497031 [details]
Reduced testcase

Reduced
Attachment #497015 - Attachment is obsolete: true

Updated

7 years ago
Summary: Use-after-free error when executing modified testcase of bug 596103 → Use-after-free error with evalcx() under gczeal
Whiteboard: [sg:nse][debug-only]
(Reporter)

Comment 7

7 years ago
Hmm... These don't seem like serious security flaws, but without gczeal, valgrind just loves evalcx(gc()).

Valgrind output attached next.
(Reporter)

Comment 8

7 years ago
Created attachment 504179 [details]
Valgrind's output, without gczeal
Attachment #497027 - Attachment is obsolete: true

Comment 9

7 years ago
Alex, it looks like you need to build the js shell with --enable-valgrind. If you run the shell with -m, you also need to pass --smc-check=all to valgrind.
(Reporter)

Comment 10

7 years ago
(In reply to comment #9)
> Alex, it looks like you need to build the js shell with --enable-valgrind. If
> you run the shell with -m, you also need to pass --smc-check=all to valgrind.

Okay, I do that with m-c builds, I'll try that with TM. Thanks!
(Reporter)

Comment 11

7 years ago
(In reply to comment #9)
> Alex, it looks like you need to build the js shell with --enable-valgrind. If
> you run the shell with -m, you also need to pass --smc-check=all to valgrind.
Suddenly, as if by magic, I cannot reproduce this. We should open this up.

Updated

7 years ago
Group: core-security
Can anyone reproduce this still with a valgrind build?
Whiteboard: [sg:nse][debug-only] → [debug-only]
(Assignee)

Updated

3 years ago
Assignee: general → nobody
You need to log in before you can comment on or make changes to this bug.