Assertion failure: tokenOffsetArg <= linebufLengthArg, at js/src/jsapi.cpp:3742 through StructuredClone
Categories
(Core :: IPC, defect, P1)
Tracking
()
People
(Reporter: decoder, Assigned: sfink)
References
Details
(5 keywords, Whiteboard: [sec-survey][post-critsmash-triage][adv-main96+r][adv-ESR91.5+r])
Attachments
(3 files)
4.87 KB,
text/plain
|
Details | |
116 bytes,
application/octet-stream
|
Details | |
48 bytes,
text/x-phabricator-request
|
diannaS
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr91+
tjr
:
sec-approval+
|
Details | Review |
Using the fuzzing target in bug 1590068 (a target that uses dom::ipc::StructuredCloneData as its entry point to cover the StructuredCloneReader code that is browser-only), I was able to find the following crash on mozilla-central revision 9b9f8bfe2625+.
I also applied the patch in bug 1736046 to unblock the fuzzing.
Backtrace:
==16686==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f0987d2f179 bp 0x7fff8f149ed0 sp 0x7fff8f149ed0 T16686)
==16686==The signal is caused by a WRITE memory access.
==16686==Hint: address points to the zero page.
#0 0x7f0987d2f179 in JSErrorReport::initBorrowedLinebuf(char16_t const*, unsigned long, unsigned long) js/src/jsapi.cpp:3742:3
#1 0x7f09856e7bae in initOwnedLinebuf dist/include/js/ErrorReport.h:266:5
#2 0x7f09856e7bae in mozilla::dom::ClonedErrorHolder::ToErrorValue(JSContext*, JS::MutableHandle<JS::Value>) dom/ipc/ClonedErrorHolder.cpp:312:16
#3 0x7f09856e7314 in mozilla::dom::ClonedErrorHolder::ReadStructuredClone(JSContext*, JSStructuredCloneReader*, mozilla::dom::StructuredCloneHolder*) dom/ipc/ClonedErrorHolder.cpp:225:43
#4 0x7f0982f0d676 in mozilla::dom::StructuredCloneHolder::CustomReadHandler(JSContext*, JSStructuredCloneReader*, JS::CloneDataPolicy const&, unsigned int, unsigned int) dom/base/StructuredCloneHolder.cpp:1005:12
#5 0x7f0987c44805 in JSStructuredCloneReader::startRead(JS::MutableHandle<JS::Value>, js::gc::InitialHeap) js/src/vm/StructuredClone.cpp:2814:11
#6 0x7f0987c348b2 in JSStructuredCloneReader::read(JS::MutableHandle<JS::Value>, unsigned long) js/src/vm/StructuredClone.cpp:3226:8
#7 0x7f0987c34671 in ReadStructuredClone(JSContext*, JSStructuredCloneData const&, JS::StructuredCloneScope, JS::MutableHandle<JS::Value>, JS::CloneDataPolicy const&, JSStructuredCloneCallbacks const*, void*) js/src/vm/StructuredClone.cpp:705:12
#8 0x7f0987c497c0 in JS_ReadStructuredClone(JSContext*, JSStructuredCloneData const&, unsigned int, JS::StructuredCloneScope, JS::MutableHandle<JS::Value>, JS::CloneDataPolicy const&, JSStructuredCloneCallbacks const*, void*) js/src/vm/StructuredClone.cpp:3382:10
#9 0x7f0982f0b8bd in mozilla::dom::StructuredCloneHolder::ReadFromBuffer(nsIGlobalObject*, JSContext*, JSStructuredCloneData&, unsigned int, JS::MutableHandle<JS::Value>, JS::CloneDataPolicy const&, mozilla::ErrorResult&) dom/base/StructuredCloneHolder.cpp:409:8
#10 0x7f0982f0b814 in mozilla::dom::StructuredCloneHolder::ReadFromBuffer(nsIGlobalObject*, JSContext*, JSStructuredCloneData&, JS::MutableHandle<JS::Value>, JS::CloneDataPolicy const&, mozilla::ErrorResult&) dom/base/StructuredCloneHolder.cpp:395:3
#11 0x7f098575b271 in mozilla::dom::ipc::StructuredCloneData::Read(JSContext*, JS::MutableHandle<JS::Value>, JS::CloneDataPolicy const&, mozilla::ErrorResult&) dom/ipc/StructuredCloneData.cpp:116:3
#12 0x7f09857559ae in mozilla::dom::ipc::StructuredCloneData::Read(JSContext*, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) dom/ipc/StructuredCloneData.cpp:104:3
#13 0x7f098087c6f5 in FuzzingRunDomSC(unsigned char const*, unsigned long) dom/base/fuzztest/FuzzStructuredClone.cpp:59:10
[...]
Marking s-s because of range assert. If this is not s-s, please keep it hidden anyway until we have completed landing of the parent target in bug 1590068.
Reporter | ||
Comment 1•3 years ago
|
||
Reporter | ||
Comment 2•3 years ago
|
||
Assignee | ||
Comment 3•3 years ago
|
||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 4•3 years ago
|
||
[Tracking Requested - why for this release]: The fuzzer that found this has been landed publicly, so we should be sure to uplift this fix.
Comment 5•3 years ago
|
||
I didn't check the code in branches, but I just assumed that this code is ancient.
Assignee | ||
Comment 6•3 years ago
|
||
Comment on attachment 9249314 [details]
Bug 1739366 - Ignore malformed cloned error
Security Approval Request
- How easily could an exploit be constructed based on the patch?: This allows arbitrary-sized out of bounds reads (and requires exploiting a content process first). I don't believe it can be used for writing.
- Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
- Which older supported branches are affected by this flaw?: all
- If not all supported branches, which bug introduced the flaw?: None
- Do you have backports for the affected branches?: No
- If not, how different, hard to create, and risky will they be?: I expect it'll graft cleanly.
- How likely is this patch to cause regressions; how much testing does it need?: low risk, won't be hit unless someone manages to corrupt a structured clone, and when it is hit it leaves the affected value unset, which is already a common code path.
Updated•3 years ago
|
Updated•3 years ago
|
Comment 7•3 years ago
|
||
Comment on attachment 9249314 [details]
Bug 1739366 - Ignore malformed cloned error
Approved to land and request uplift
Updated•3 years ago
|
Comment 8•3 years ago
|
||
Comment 9•3 years ago
|
||
As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.
Please visit this google form to reply.
Updated•3 years ago
|
Comment 10•3 years ago
|
||
Steve, do you want to uplift this to beta?
Comment 11•3 years ago
|
||
Comment on attachment 9249314 [details]
Bug 1739366 - Ignore malformed cloned error
Beta/Release Uplift Approval Request
- User impact if declined: possible security issue
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): see the answer to the last question in comment 6
- String changes made/needed: none
Comment 12•3 years ago
|
||
Comment on attachment 9249314 [details]
Bug 1739366 - Ignore malformed cloned error
Approved for 96.0b9
Comment 13•3 years ago
|
||
uplift |
Assignee | ||
Updated•3 years ago
|
Comment 14•3 years ago
|
||
Comment on attachment 9249314 [details]
Bug 1739366 - Ignore malformed cloned error
Approved for 91.5esr also.
Updated•3 years ago
|
Comment 15•3 years ago
|
||
uplift |
Updated•2 years ago
|
Description
•