Closed
Bug 1477329
Opened 3 years ago
Closed 3 years ago
ASan: WasmTextToBinary reinterpret a Vector<wasm::AstName> to a Vector<wasm::AstValType>.
Categories
(Core :: Javascript: WebAssembly, defect)
Core
Javascript: WebAssembly
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: nbp, Assigned: lth)
References
Details
(Whiteboard: [js shell only])
Attachments
(1 file)
3.84 KB,
patch
|
jseward
:
review+
|
Details | Diff | Splinter Review |
This issue can be reproduced with the patch attached to Bug 1467116 with ASan and either debug/optimized build, and by running the test case wasm/gc/ref.js with --wasm-gc ==10734==ERROR: AddressSanitizer: use-after-poison on address 0x6210003aef30 at pc 0x0000033a6ddf bp 0x7ffd94a916c0 sp 0x7ffd94a916b0 #14 0x00000000033a6ddf in js::wasm::AstValType::code (this=0x6210003aef30) at /home/nicolas/mozilla/wksp-6/js/src/wasm/WasmAST.h:161 #15 0x00000000033a6979 in js::wasm::AstFuncType::hash (ft=...) at /home/nicolas/mozilla/wksp-6/js/src/wasm/WasmAST.h:376 #16 0x00000000033a5be5 in js::detail::HashTable<js::HashMapEntry<js::wasm::AstFuncType*, unsigned int>, js::HashMap<js::wasm::AstFuncType*, unsigned int, js::wasm::AstFuncType, js::LifoAllocPolicy<(js::Fallibility)0> >::MapHashPolicy, js::LifoAllocPolicy<(js::Fallibility)0> >::prepareHash (l=...) at /home/nicolas/mozilla/_build/js/bugzil.la/1470115/wip/x64/clang/dbg/dist/include/js/HashTable.h:1284 #17 0x00000000033a58b0 in js::detail::HashTable<js::HashMapEntry<js::wasm::AstFuncType*, unsigned int>, js::HashMap<js::wasm::AstFuncType*, unsigned int, js::wasm::AstFuncType, js::LifoAllocPolicy<(js::Fallibility)0> >::MapHashPolicy, js::LifoAllocPolicy<(js::Fallibility)0> >::lookupForAdd (this=0x6210003ae958, l=...) at /home/nicolas/mozilla/_build/js/bugzil.la/1470115/wip/x64/clang/dbg/dist/include/js/HashTable.h:1839 #18 0x00000000033a3e74 in js::HashMap<js::wasm::AstFuncType*, unsigned int, js::wasm::AstFuncType, js::LifoAllocPolicy<(js::Fallibility)0> >::lookupForAdd (this=0x6210003ae958, l=...) at /home/nicolas/mozilla/_build/js/bugzil.la/1470115/wip/x64/clang/dbg/dist/include/js/HashTable.h:152 #19 0x0000000003393927 in js::wasm::AstModule::append (this=0x6210003ae920, funcType=0x6210003aefa8) at /home/nicolas/mozilla/wksp-6/js/src/wasm/WasmAST.h:1322 #20 0x0000000003321c96 in ParseModule ( text=0x6210003bed00 u"(module\n (type $cons (struct\n", ' ' <repeats 19 times>, "(field $car i32)\n", ' ' <repeats 19 times>, "(field $cdr (ref $cons))))\n\n (type $odd (struct\n", ' ' <repeats 18 times>, "(field $x i32)\n", ' ' <repeats 18 times>, "(field "..., stackLimit=140727095535361, lifo=..., error=0x7ffd94a92410, binary=0x7ffd94a92050) at /home/nicolas/mozilla/wksp-6/js/src/wasm/WasmTextToBinary.cpp:4049 #21 0x00000000033214c2 in js::wasm::TextToBinary ( text=0x6210003bed00 u"(module\n (type $cons (struct\n", ' ' <repeats 19 times>, "(field $car i32)\n", ' ' <repeats 19 times>, "(field $cdr (ref $cons))))\n\n (type $odd (struct\n", ' ' <repeats 18 times>, "(field $x i32)\n", ' ' <repeats 18 times>, "(field "..., stackLimit=140727095535361, bytes=0x7ffd94a923c0, offsets=0x7ffd94a92430, error=0x7ffd94a92410) at /home/nicolas/mozilla/wksp-6/js/src/wasm/WasmTextToBinary.cpp:5937 #22 0x0000000001c80bad in WasmTextToBinary (cx=0x7ffd94a921a0, argc=1, vp=0x6210003a5d98) at /home/nicolas/mozilla/wksp-6/js/src/builtin/TestingFunctions.cpp:676 (rr) frame 15 (rr) ptype ft.args_ type = class mozilla::Vector<js::wasm::AstValType, 0, js::LifoAllocPolicy<js::Fallibility::Fallible> > Setting a breakpoint in the function which returns the allocated vector returns: Thread 1 hit Breakpoint 1, js::detail::BumpChunk::tryAlloc (this=0x6210003ae900, n=32) at /home/nicolas/mozilla/wksp-6/js/src/ds/LifoAlloc.h:460 460 uint8_t* newBump = nextAllocEnd(aligned, n); (rr) bt #0 js::detail::BumpChunk::tryAlloc (this=0x6210003ae900, n=32) at /home/nicolas/mozilla/wksp-6/js/src/ds/LifoAlloc.h:460 #1 0x0000000000ad2d4d in js::LifoAlloc::allocImpl (this=0x7ffd94a91fe0, n=32) at /home/nicolas/mozilla/wksp-6/js/src/ds/LifoAlloc.h:603 #2 0x0000000000ad2be2 in js::LifoAlloc::alloc (this=0x7ffd94a91fe0, n=32) at /home/nicolas/mozilla/wksp-6/js/src/ds/LifoAlloc.h:676 #3 0x000000000339a697 in js::LifoAllocPolicy<(js::Fallibility)0>::maybe_pod_malloc<js::wasm::AstName> (this=0x7ffd94a91300, numElems=2) at /home/nicolas/mozilla/wksp-6/js/src/ds/LifoAlloc.h:1028 #4 0x000000000339a25d in js::LifoAllocPolicy<(js::Fallibility)0>::pod_malloc<js::wasm::AstName> (this=0x7ffd94a91300, numElems=2) at /home/nicolas/mozilla/wksp-6/js/src/ds/LifoAlloc.h:1050 #5 0x000000000339a098 in mozilla::detail::VectorImpl<js::wasm::AstName, 0ul, js::LifoAllocPolicy<(js::Fallibility)0>, false>::growTo (aV=..., aNewCap=2) at /home/nicolas/mozilla/_build/js/bugzil.la/1470115/wip/x64/clang/dbg/dist/include/mozilla/Vector.h:137 #6 0x00000000033999be in mozilla::Vector<js::wasm::AstName, 0ul, js::LifoAllocPolicy<(js::Fallibility)0> >::growStorageBy (this=0x7ffd94a91300, aIncr=1) at /home/nicolas/mozilla/_build/js/bugzil.la/1470115/wip/x64/clang/dbg/dist/include/mozilla/Vector.h:1054 #7 0x00000000033a02a5 in mozilla::Vector<js::wasm::AstName, 0ul, js::LifoAllocPolicy<(js::Fallibility)0> >::append<js::wasm::AstName&> (this=0x7ffd94a91300, aU=...) at /home/nicolas/mozilla/_build/js/bugzil.la/1470115/wip/x64/clang/dbg/dist/include/mozilla/Vector.h:1409 #8 0x000000000336023a in ParseStructFields (c=..., st=0x7ffd94a91830) at /home/nicolas/mozilla/wksp-6/js/src/wasm/WasmTextToBinary.cpp:3426 #9 0x0000000003343124 in ParseTypeDef (c=...) at /home/nicolas/mozilla/wksp-6/js/src/wasm/WasmTextToBinary.cpp:3455 #10 0x0000000003321c51 in ParseModule ( text=0x6210003bed00 u"(module\n (type $cons (struct\n", ' ' <repeats 19 times>, "(field $car i32)\n", ' ' <repeats 19 times>, "(field $cdr (ref $cons))))\n\n (type $odd (struct\n", ' ' <repeats 18 times>, "(field $x i32)\n", ' ' <repeats 18 times>, "(field "..., stackLimit=140727095535361, lifo=..., error=0x7ffd94a92410, binary=0x7ffd94a92050) at /home/nicolas/mozilla/wksp-6/js/src/wasm/WasmTextToBinary.cpp:4046 #11 0x00000000033214c2 in js::wasm::TextToBinary ( text=0x6210003bed00 u"(module\n (type $cons (struct\n", ' ' <repeats 19 times>, "(field $car i32)\n", ' ' <repeats 19 times>, "(field $cdr (ref $cons))))\n\n (type $odd (struct\n", ' ' <repeats 18 times>, "(field $x i32)\n", ' ' <repeats 18 times>, "(field "..., stackLimit=140727095535361, bytes=0x7ffd94a923c0, offsets=0x7ffd94a92430, error=0x7ffd94a92410) at /home/nicolas/mozilla/wksp-6/js/src/wasm/WasmTextToBinary.cpp:5937 #12 0x0000000001c80bad in WasmTextToBinary (cx=0x7ffd94a921a0, argc=1, vp=0x6210003a5d98) at /home/nicolas/mozilla/wksp-6/js/src/builtin/TestingFunctions.cpp:676 Somehow, the vector got allocated as Vector<js::wasm::AstName> and is being used as Vector<js::wasm::AstValType> which has a different size for its elements.
Reporter | ||
Comment 1•3 years ago
|
||
The problem is that ParseTypeDef creates a new type with: type = new(c.lifo) AstStructType(name, std::move(st)); returns it to ParseModule (case WasmToken::Type), and always do a static cast into an AstFuncType.
Flags: needinfo?(lhansen)
Assignee | ||
Comment 2•3 years ago
|
||
I will fix.
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Flags: needinfo?(lhansen)
Updated•3 years ago
|
Keywords: sec-moderate
Assignee | ||
Comment 3•3 years ago
|
||
The problem here really was that we allowed static_cast to be used in the first place, when we already have checked type transfer functions that should be used. Probably a consequence of evolving the code over time. I just removed static_cast everywhere and introduced a helper for append(), which is somewhat redundant since it only has one caller, but it's clean.
Attachment #8994418 -
Flags: review?(jseward)
Assignee | ||
Comment 4•3 years ago
|
||
Looks like this will need to be uplifted to beta.
Comment 5•3 years ago
|
||
Comment on attachment 8994418 [details] [diff] [review] bug1477329-no-static-cast.patch LGTM, + restores V-cleanness in the test case from dup bug 1477913.
Attachment #8994418 -
Flags: review?(jseward) → review+
Assignee | ||
Comment 6•3 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a095dcb31ff53d06ed8ea61e592b6477fcb97cb
Assignee | ||
Comment 7•3 years ago
|
||
Actually, a beta uplift is not necessary. The bug is in the text-to-binary functionality which is only in the JS shell, not in the browser. Daniel, can you open this bug? It is not a security concern.
Flags: needinfo?(dveditz)
https://hg.mozilla.org/mozilla-central/rev/0a095dcb31ff
Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•3 years ago
|
Updated•3 years ago
|
Group: core-security-release
Whiteboard: [jsshell only → [js shell only]
You need to log in
before you can comment on or make changes to this bug.
Description
•