Closed Bug 1652854 Opened 1 year ago Closed 1 year ago

AddressSanitizer: SEGV or Crash [@ js::SetObject::trace] or Assertion failure: !arr->getDenseElement(i).isMagic(JS_ELEMENTS_HOLE), at vm/NativeObject-inl.h:855

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla80
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox78 --- unaffected
firefox79 --- unaffected
firefox80 --- fixed

People

(Reporter: gkw, Assigned: jandem)

References

(Regression)

Details

(Keywords: regression, testcase)

Crash Data

Attachments

(6 files)

x = [];
Object.defineProperty(x, 17, {
  configurable: true,
  enumerable: true,
  writable: true,
});
Object.defineProperty(this, "y", {
  get: function () {
    return x.slice();
  }
});
y.shift();

Compiled using GCC 9.3.0 and Clang 9 with:

AR=ar sh ./configure --enable-debug --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

Run with:

--fuzzing-safe --no-threads --no-baseline --no-ion

Tested on m-c rev bca48c382991..

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/f112125cab4d
user:        Jan de Mooij
date:        Tue Jul 14 07:02:11 2020 +0000
summary:     Bug 1651645 part 9 - Add NON_PACKED flag to ObjectElements, use it in IsPackedArray. r=evilpie,anba

Unsure if this is a bad assertion failure, deferring to :jandem. ASan opt builds don't show anything.

Flags: sec-bounty?
Flags: needinfo?(jdemooij)

LangFuzz found the same bug, see also bug 1652875.

See Also: → 1652875
Group: core-security → javascript-core-security

Good find. The TI-based code relied on CopyDenseArrayElements => NewFullyAllocatedArrayTryReuseGroup either reusing the group or using a group with bad type information, so we didn't have to propagate the flag when memcpy'ing the elements.

Attached file ASan stack
x = new Set([, 0].slice())

Run with: --fuzzing-safe --no-threads --no-baseline --no-ion on m-c rev d31bc01c978f.

Here's an even smaller, even better testcase that causes an ASan error on opt builds. It shows the same assertion failure on debug builds too.

With all due respect, I couldn't get my original testcase in comment 0 nor :decoder's testcase in bug 1652875 to cause an ASan error on opt builds, but perhaps the fuzzing infrastructure has already found something.

(This is partially why I believe it is important to consider retaining split bounties on ties found by MoCo internal infra within first 3 days / 72 hours - it inspired me to poke more at this issue)

Crash Signature: [@ js::SetObject::trace]
Summary: Assertion failure: !arr->getDenseElement(i).isMagic(JS_ELEMENTS_HOLE), at vm/NativeObject-inl.h:855 → AddressSanitizer: SEGV or Crash [@ js::SetObject::trace] or Assertion failure: !arr->getDenseElement(i).isMagic(JS_ELEMENTS_HOLE), at vm/NativeObject-inl.h:855

(In reply to Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) from comment #4)

Here's an even smaller, even better testcase that causes an ASan error on opt builds.

The ASan error you attached is not an ASan error but hitting a release assertion.

Attached image screenshot.jpg

Nonetheless, the testcase in comment 4 also crashes in latest ASan Windows builds by wrapping in a <script></script> tag.

Downloaded via https://firefox-source-docs.mozilla.org/tools/sanitizer/asan.html#downloading-artifact-builds from https://firefox-ci-tc.services.mozilla.com/api/index/v1/task/gecko.v2.mozilla-central.latest.firefox.win64-asan-opt/artifacts/public/build/target.zip (also shows the crash on Linux). Takes a few seconds.

I don't remember why I currently can't find about:crashes in this ASan build though - perhaps I'm doing something wrong? How would people report ASan bugs by running ASan builds?

I also added a jit-test for the IsPackedArray intrinsic. Now that this doesn't
depend on TI anymore, the behavior should be more predictable and this way we
can make sure we don't regress/break the packed-array optimization.

Assignee: nobody → jdemooij
Status: NEW → ASSIGNED

I did a quick audit and I suspect this will either hit the release-assert in Value::isMagic, crash at a near-null address, or have non-exploitable correctness bugs.

Flags: needinfo?(jdemooij)
Duplicate of this bug: 1652875
Severity: -- → S3
Priority: -- → P1
Duplicate of this bug: 1652963
Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80

Awarding a split bounty with bug 1652875 and bug 1652963 under the rules existing at the time. This cluster of bugs were all filed within a day of the regressing bug landing and in the future that will fall within our 4-day exclusion window.

Flags: sec-bounty? → sec-bounty+
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.