Possible crash in AvmCore::throwErrorV() in non-debugger builds

RESOLVED FIXED in Q3 11 - Serrano

Status

Tamarin
Virtual Machine
P2
normal
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: William Maddox, Assigned: Lars T Hansen)

Tracking

(Blocks: 1 bug)

unspecified
Q3 11 - Serrano
Bug Flags:
flashplayer-qrb +
flashplayer-bug +

Details

(Whiteboard: Has patch)

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
The code below will dereference NULL in a non-debugger build should type == NULL.  This is presumably an erroneous condition, but one that is allowed for.  We should at least cleanly terminate.

This issue was detected by static analysis and does not reflect an actual observed failure.

    void AvmCore::throwErrorV(ClassClosure *type, int errorID, Stringp arg1, Stringp arg2, Stringp arg3)
    {
        Stringp out = formatErrorMessageV( errorID, arg1, arg2, arg3);

        #ifdef DEBUGGER
        if (type == NULL)
        {
            // print the error message, because we're still bootstrapping
            // and the exception type is not yet defined
            console << out << "\n";
        }
        #endif

        Atom args[3] = { nullObjectAtom, out->atom(), intToAtom(errorID) };
        throwAtom(type->construct(2, args));
    }

Updated

8 years ago
Blocks: 551176

Comment 1

8 years ago
even in DEBUGGER, it will dereference a NULL type.  The erroneous condition is only possible when the VM is bootstrapping itself, indicating a bug in the VM itself and not something a working VM could ever experience at runtime.  Ideally the whole #ifdef DEBUGGER block should be turned into an AvmAssertMsg.

Updated

8 years ago
Flags: flashplayer-qrb+
Priority: -- → P2
Target Milestone: --- → flash10.1

Comment 2

8 years ago
Retargeting to 10.2, not a critical issue.
Target Milestone: flash10.1 → flash10.2
(Assignee)

Updated

8 years ago
Assignee: nobody → lhansen
(Assignee)

Comment 3

8 years ago
Created attachment 438140 [details] [diff] [review]
Patch
Attachment #438140 - Flags: review?(edwsmith)
(Assignee)

Updated

8 years ago
Status: NEW → ASSIGNED
Whiteboard: Has patch

Updated

8 years ago
Attachment #438140 - Flags: review?(edwsmith) → review+
(Assignee)

Comment 4

8 years ago
tamarin-redux changeset:   4384:95d7dd9119aa
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

7 years ago
Flags: flashplayer-bug+
You need to log in before you can comment on or make changes to this bug.