Open Bug 967176 Opened 6 years ago Updated 6 years ago

Faulty: Assertion failure: (nbytes & 7) == 0, at js/src/vm/StructuredClone.cpp:414

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

Tracking

()

People

(Reporter: bjacob, Assigned: jorendorff)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached file Faulty session
Found by Christoph Diehl's "Faulty" fuzzer, see bug 777067

Reproduces on desktop linux debug build with tabs.remote pref, with the Faulty IPC fuzzer, using this environment:

FAULTY_SEED=3 FAULTY_PICKLE=1 FAULTY_PARENT=1 FAULTY_ENABLE_LOGGING=1 FAULTY_PROBABILITY=10
Classification: non-gfx, needs JS people to look at it
Ben, whom should we ask to look into this?
Flags: needinfo?(bent.mozilla)
Hi Jason, we're starting to fuzz messages that come from child processes (for B2G especially, but this is vital for electrolysis too). If a hacked child process sends bad structured clone data we're at least seeing this one assertion. Who would be able to look into the structured clone code to make sure that the parent process can't be compromised in this way?
Flags: needinfo?(bent.mozilla) → needinfo?(jorendorff)
CC'ing sfink and bhackett too since they've at least looked at the structured clone code before.
The JS engine is not doing anything wrong here, we should just not make it deal with something which violates its assumptions.
Assignee: nobody → ehsan
Component: JavaScript Engine → DOM
Attachment #8371158 - Flags: review?(bzbarsky)
Comment on attachment 8371158 [details] [diff] [review]
Protect against some of the values that invalidate the JS engine assumptions when reading structured clones; r=bzbarsky

There are a lot more places than just this that read in structured clone data from a potentially untrusted source (the IPC wire, the disk, etc).

I really think the JS engine should just deal here.
Attachment #8371158 - Flags: review?(bzbarsky) → review-
Fair enough!  (and thanks for the review)
Assignee: ehsan → nobody
Component: DOM → JavaScript Engine
Agreed. Taking.

The serialization format here was *originally* designed to be very nicely behaved in the face of malicious data: report an error and return false; don't assert, don't abort, and by no means trigger undefined behavior.

But we have drifted away from this. For example, this line in js/src/vm/StructuredClone.cpp
        MOZ_ASSUME_UNREACHABLE("unknown TypedArrayObject type");
should be replaced with a JS_ReportErrorNumber call.

I'll document this requirement and review the code to make sure we implement it, as before.
Assignee: nobody → jorendorff
Flags: needinfo?(jorendorff)
You need to log in before you can comment on or make changes to this bug.