1528794 - Assertion failure: tag == SCTAG_ARRAY_BUFFER_OBJECT, at js/src/vm/StructuredClone.cpp:2712

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 7ab4a0c9980f (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe):

let data = new Uint8Array([
  0x02, 0xd5, 0x00, 0x22, 0x00, 0x02, 0xff, 0xff, 0xff, 0xf7, 0xff, 0xff,
  0xff, 0xdf, 0xff, 0xff, 0xff, 0x5f, 0xff, 0x32, 0x03, 0x02, 0xff, 0xff,
  0x34, 0xff, 0x72, 0x65, 0x6c, 0x6f, 0x61, 0x64, 0x0b, 0x0, 0x0, 0x0, 
  0x0, 0x0, 0x0, 0x0
]);
let cloneBuffer = serialize(null);
cloneBuffer.clonebuffer = data.buffer;
deserialize(cloneBuffer);

Backtrace:

received signal SIGSEGV, Segmentation fault.
JSStructuredCloneReader::readTransferMap (this=this@entry=0x7fffffffc4f0) at js/src/vm/StructuredClone.cpp:2712
#0  JSStructuredCloneReader::readTransferMap (this=this@entry=0x7fffffffc4f0) at js/src/vm/StructuredClone.cpp:2712
#1  0x0000555555ce9d48 in JSStructuredCloneReader::read (this=this@entry=0x7fffffffc4f0, vp=vp@entry=...) at js/src/vm/StructuredClone.cpp:2857
#2  0x0000555555cea687 in ReadStructuredClone (cx=<optimized out>, data=..., scope=scope@entry=JS::StructuredCloneScope::DifferentProcess, vp=..., cb=cb@entry=0x0, cbClosure=cbClosure@entry=0x0) at js/src/vm/StructuredClone.cpp:637
#3  0x0000555555cea7f7 in JS_ReadStructuredClone (cx=<optimized out>, buf=..., version=version@entry=8, scope=scope@entry=JS::StructuredCloneScope::DifferentProcess, vp=..., vp@entry=..., optionalCallbacks=optionalCallbacks@entry=0x0, closure=0x0) at js/src/vm/StructuredClone.cpp:2992
#4  0x0000555555c58918 in Deserialize (cx=<optimized out>, cx@entry=0x7ffff5f17000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TestingFunctions.cpp:3179
#5  0x00005555558f4cc9 in CallJSNative (cx=0x7ffff5f17000, native=0x555555c58550 <Deserialize(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:440
[...]
#19 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:10970
rax	0x555557c27280	93825032942208
rbx	0x7fffffffc200	140737488339456
rcx	0x555556b7e448	93825015473224
rdx	0x0	0
rsi	0x7ffff6eeb770	140737336227696
rdi	0x7ffff6eea540	140737336223040
rbp	0x7fffffffc2c0	140737488339648
rsp	0x7fffffffc170	140737488339312
r8	0x7ffff6eeb770	140737336227696
r9	0x7ffff7fe6cc0	140737354034368
r10	0x58	88
r11	0x7ffff6b927a0	140737332717472
r12	0x7fffffffc4f0	140737488340208
r13	0xffff020332ff5fff	-279262212956161
r14	0x7fffffffc260	140737488339552
r15	0x7ffff5f17000	140737319628800
rip	0x555555ce7c39 <JSStructuredCloneReader::readTransferMap()+1945>
=> 0x555555ce7c39 <JSStructuredCloneReader::readTransferMap()+1945>:	movl   $0x0,0x0
   0x555555ce7c44 <JSStructuredCloneReader::readTransferMap()+1956>:	ud2

Marking s-s because this is a potentially dangerous assert in StructuredClone. This was originally found by the libFuzzer target, but I converted it to JS to try and figure out what it going on. We didn't pick this up earlier because it doesn't crash in opt builds.

Comment 1

[Christian Holler (:decoder)]

Assuming that this is responsible for the recent slows/ooms in SC fuzzing, I'll mark this as a fuzzblocker. Needinfo for :sfink as a start as he works on structured cloning.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/85839dc95172
user: Steve Fink
date: Mon Oct 09 17:41:01 2017 -0700
summary: Bug 1406508 - Allow fuzzers to set binary clone buffer data, r=jorendorff

Steve, is bug 1406508 a likely regressor?

Comment 3

[Alex Gaynor [:Alex_Gaynor]]

I don't think this needs to be s-s, it looks like the assertion is to verify that the sending-side doesn't have a bug, but the code is still correct/memory safe without it.

Comment 4

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/85839dc95172
user:        Steve Fink
date:        Mon Oct 09 17:41:01 2017 -0700
summary:     Bug 1406508 - Allow fuzzers to set binary clone buffer data, r=jorendorff

This iteration took 268.736 seconds to run.

Comment 5

[Steve Fink [:sfink] [:s:]]

Bleh, there are numerous ways for this to assert.

(In reply to Gary Kwong [:gkw] [:nth10sd] - gradually taking PTO-ish till Mar 11 from comment #2)

Steve, is bug 1406508 a likely regressor?

Yes.

Although the scope is DifferentProcess, which should prevent any of this from causing harm, this code is assuming valid clone data. I'll need to fix several places.

Comment 6

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1528794 - Check clone buffer contents at runtime r=jorendorff

Comment 7

[Steve Fink [:sfink] [:s:]]

Actually, this ended up being pretty minimal.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0b61149893f9
Check clone buffer contents at runtime r=jorendorff

Comment 9

[Dorel Luca [:dluca]]

Backed out changeset 0b61149893f9 (bug 1528794) for build bustage. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=231394530&repo=autoland&lineNumber=34659

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=0b61149893f94fcc65fe6535c55f6ef6240741db

Backout:
https://hg.mozilla.org/integration/autoland/rev/689c3fe760069f90a24039a4d24986f0d38303ca

Comment 10

[Jason Orendorff [:jorendorff]]

Easy fix. Sorry I missed it on review. (bashful look)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a9911c4882de
Check clone buffer contents at runtime r=jorendorff

Comment 12

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/a9911c4882de

Comment 13

[Ryan VanderMeulen [:RyanVM]]

I'm assuming we don't need to worry about backporting this since RC week is next week, but can/should we land the test from this still?

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Guess not.

Comment 15

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1528794 - Regression test for invalid binary structured clone data

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e88adaf3b105
Regression test for invalid binary structured clone data

Comment 17

[Steve Fink [:sfink] [:s:]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #13)

I'm assuming we don't need to worry about backporting this since RC week is next week, but can/should we land the test from this still?

Uh, yeah, right. I really shouldn't ignore my needinfos for multiple years, I guess.

I landed the test.

Comment 18

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4fd5c458be4c
Backed out changeset e88adaf3b105 for causing jit failures instructured-clone/regress1528794.js