Closed Bug 1823379 (CVE-2023-32211) Opened 1 year ago Closed 1 year ago

Incorrect validation for repeated catch blocks

Categories

(Core :: JavaScript: WebAssembly, defect, P1)

Product:
Core
Component:
JavaScript: WebAssembly
Platform:
All
Unspecified
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
114 Branch
Tracking Flags:
Tracking Status
firefox-esr102 113+ fixed
firefox111 --- wontfix
firefox112 --- wontfix
firefox113 + fixed
firefox114 + fixed

People

(Reporter: cz18811105578, Assigned: rhunt)

References

(Regression)

Details

(Keywords: regression, sec-moderate, Whiteboard: [adv-main113+][adv-ESR102.11+])

Attachments

(4 files)

poc.js
941 bytes, text/javascript
Details
Bug 1823379. r=jseward
48 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
RyanVM
: approval-mozilla-esr102+
tjr
: sec-approval+
Details | Review
Bug 1823379 - wasm: Test case. r?jseward
48 bytes, text/x-phabricator-request
Details | Review
advisory.txt
137 bytes, text/plain
Details
Attached file poc.js —

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/111.0.0.0 Safari/537.36

Steps to reproduce:

The vulnerability exists in the Javascript engine under the ARM64 architecture, so it needs to be reproduced in the gecko in ARM64. The configuration for building is as follows:

ac_add_options --enable-fuzzing
ac_add_options --enable-gczeal
ac_add_options --enable-project=js
ac_add_options --enable-optimize
ac_add_options --enable-debug

Commit: 8ed22fcd56968c95a73a6c82b42f732f01a4bdae

Actual results:

This is the full crash log of the debug build:

UndefinedBehaviorSanitizer:DEADLYSIGNAL
==15739==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000024 (pc 0x000103a5abbc bp 0x00016db0ead0 sp 0x00016db0ea70 T644387)
==15739==The signal is caused by a READ memory access.
==15739==Hint: address points to the zero page.
    #0 0x103a5abbc in (anonymous namespace)::FunctionCompiler::checkOffsetAndAlignmentAndBounds(js::wasm::MemoryAccessDesc*, js::jit::MDefinition**)+0xd0 (js:arm64+0x10176ebbc) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #1 0x103a5c2d8 in (anonymous namespace)::FunctionCompiler::store(js::jit::MDefinition*, js::wasm::MemoryAccessDesc*, js::jit::MDefinition*)+0x134 (js:arm64+0x1017702d8) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #2 0x103a173e0 in EmitStore((anonymous namespace)::FunctionCompiler&, js::wasm::PackedType<js::wasm::ValTypeTraits>, JS::Scalar::Type)+0x2a0 (js:arm64+0x10172b3e0) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #3 0x1039c2700 in EmitBodyExprs((anonymous namespace)::FunctionCompiler&)+0x9cc (js:arm64+0x1016d6700) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #4 0x1039c08fc in js::wasm::IonCompileFunctions(js::wasm::ModuleEnvironment const&, js::wasm::CompilerEnvironment const&, js::LifoAlloc&, mozilla::Vector<js::wasm::FuncCompileInput, 8ul, js::SystemAllocPolicy> const&, js::wasm::CompiledCode*, mozilla::UniquePtr<char [], JS::FreePolicy>*)+0xdb4 (js:arm64+0x1016d48fc) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #5 0x10396e914 in ExecuteCompileTask(js::wasm::CompileTask*, mozilla::UniquePtr<char [], JS::FreePolicy>*)+0x19c (js:arm64+0x101682914) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #6 0x103970870 in js::wasm::ModuleGenerator::finishFuncDefs()+0x40 (js:arm64+0x101684870) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #7 0x10393d138 in bool DecodeCodeSection<js::wasm::Decoder>(js::wasm::ModuleEnvironment const&, js::wasm::Decoder&, js::wasm::ModuleGenerator&)+0x44c (js:arm64+0x101651138) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #8 0x10393cbd4 in js::wasm::CompileBuffer(js::wasm::CompileArgs const&, js::wasm::ShareableBytes const&, mozilla::UniquePtr<char [], JS::FreePolicy>*, mozilla::Vector<mozilla::UniquePtr<char [], JS::FreePolicy>, 0ul, js::SystemAllocPolicy>*, JS::OptimizedEncodingListener*)+0x300 (js:arm64+0x101650bd4) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #9 0x1039d8f60 in js::WasmModuleObject::construct(JSContext*, unsigned int, JS::Value*)+0x21c (js:arm64+0x1016ecf60) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #10 0x1023cee90 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&)+0x1a4 (js:arm64+0x1000e2e90) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #11 0x1023ddf00 in CallJSNativeConstructor(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&)+0xa4 (js:arm64+0x1000f1f00) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #12 0x1023d0400 in InternalConstruct(JSContext*, js::AnyConstructArgs const&, js::CallReason)+0x368 (js:arm64+0x1000e4400) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #13 0x1023c5c74 in Interpret(JSContext*, js::RunState&)+0x6830 (js:arm64+0x1000d9c74) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #14 0x1023bf030 in js::RunScript(JSContext*, js::RunState&)+0x1e8 (js:arm64+0x1000d3030) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #15 0x1023d111c in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>)+0x288 (js:arm64+0x1000e511c) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #16 0x1023d1624 in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>)+0x200 (js:arm64+0x1000e5624) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #17 0x10251df58 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>)+0x100 (js:arm64+0x100231f58) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #18 0x10251e160 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>)+0xd8 (js:arm64+0x100232160) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #19 0x102358d30 in RunFile(JSContext*, char const*, __sFILE*, CompileUtf8, bool, bool)+0x450 (js:arm64+0x10006cd30) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #20 0x1023582ac in Process(JSContext*, char const*, bool, FileKind)+0x8a4 (js:arm64+0x10006c2ac) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #21 0x1022fc88c in Shell(JSContext*, js::cli::OptionParser*)+0x115c (js:arm64+0x10001088c) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #22 0x1022f6d48 in main+0x690 (js:arm64+0x10000ad48) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00)
    #23 0x195867e4c  (<unknown module>)
    #24 0xda46fffffffffffc  (<unknown module>)

==15739==Register values:
 x[0] = 0x0000000000000001   x[1] = 0x0000000000000001   x[2] = 0x000000016db0eae0   x[3] = 0x0000000000000000
 x[4] = 0x0000000105413d3c   x[5] = 0x00000001039c26f4   x[6] = 0x000000010409ad00   x[7] = 0x000000016db0ea18
 x[8] = 0x00000001054173e7   x[9] = 0x0000000000000001  x[10] = 0x00000001054173e7  x[11] = 0x0000000002000000
x[12] = 0xf8000000ffffffff  x[13] = 0x0000000000000001  x[14] = 0x0000000000000001  x[15] = 0x000000010409b8c4
x[16] = 0x0000000103a8d188  x[17] = 0x00000000000002a1  x[18] = 0x0000000000000000  x[19] = 0x000000016db0eae0
x[20] = 0x000000016db0f020  x[21] = 0x000000016db0eb38  x[22] = 0x0000000000000000  x[23] = 0x0000000002000000
x[24] = 0xaaaaaaaaaaaa0039  x[25] = 0x0000000000000039  x[26] = 0x000000016db0f954  x[27] = 0xaaaaaaaaaaaaaaaa
x[28] = 0x0000000105413d3c     fp = 0x000000016db0ead0     lr = 0x0000000103a5ab78     sp = 0x000000016db0ea70
UndefinedBehaviorSanitizer can not provide additional info.
SUMMARY: UndefinedBehaviorSanitizer: SEGV (js:arm64+0x10176ebbc) (BuildId: 4c4c44b255553144a1deaf01cdd3b98d32000000200000000100000000000b00) in (anonymous namespace)::FunctionCompiler::checkOffsetAndAlignmentAndBounds(js::wasm::MemoryAccessDesc*, js::jit::MDefinition**)+0xd0
==15739==ABORTING
Attachment #9323912 - Attachment description: 2.js → poc.js
Group: firefox-core-security → core-security
Component: Untriaged → JavaScript: WebAssembly
Product: Firefox → Core
Hardware: Unspecified → ARM64

This is reported by P1umer and xmzyshypnc

Group: core-security → javascript-core-security
See Also: → 1823384
Assignee: nobody → jseward
Severity: -- → S3
Priority: -- → P2

Talked about this with Julian, this looks to be a type checking bug that is causing us to attempt to compile invalid code. Usually this fails because of assertions or MIR issues, but theoretically could cause issues. We're working on a fix and should have an assessment on the risk soon.

Severity: S3 → S2
Priority: P2 → P1
See Also: → 1823330

See also bug 1826292 for further commentary.

See Also: → 1826292
Attached file Bug 1823379. r=jseward —

catch is a combined block terminator and block beginning
instruction. So it keeps the top 'control stack entry' but must
change the label kind and reset the polymorphicBase flag (which
is set by unreachable code). We did this correctly for the
first catch after a try, but missed it for repeated
catch blocks.

This commit also has a driveby fix for tracking whether
a non-nullable local (from the function references proposal)
is initialized or not. This has the same problem as unreachable
code.

Duplicate of this bug: 1823384
Duplicate of this bug: 1823330
Duplicate of this bug: 1826292

This bug is original to the first implementation of wasm exception handling which rode the trains in 100 (bug 1759217).

Assignee: jseward → rhunt
status-firefox111: --- → affected
status-firefox112: --- → affected
status-firefox113: --- → affected
status-firefox-esr102: --- → affected
Hardware: ARM64 → All
Summary: [WASM] SEGV in checkOffsetAndAlignmentAndBounds → Incorrect validation for repeated catch blocks

I'm not sure the severity here. We have an incorrect validation algorithm for an 'unreachable' statement in a 'catch' clause followed by another 'catch' clause. This causes invalid code to be attempted to be JIT compiled.

In Ion this causes the compiler to receive null MDefinition* objects as parameters to future MIR nodes, which causes a nullptr SEGV down the road. For baseline, it looks like you can get the compiler to attempt to pop values from it's internal representation of the wasm stack but I cannot seem to get around some release assertions to get a miscompiled function instead of a crash. Doesn't mean it's not possible, I might just not be creative enough.

Conservatively, this is a bad issue and I can't rule out someone being able to miscompile functions with baseline in a bad way.

The following field has been copied from a duplicate bug:

Field Value Source
Status NEW bug 1826292

For more information, please visit auto_nag documentation.

Status: UNCONFIRMED → NEW
Ever confirmed: true

Posting the commit message here for posterity:

`catch` is a combined block terminator and block beginning
instruction. So it keeps the top 'control stack entry' but must
change the label kind and reset the polymorphicBase flag (which
is set by unreachable code). We did this correctly for the
first `catch` after a `try`, but missed it for repeated
`catch` blocks.

This commit also has a driveby fix for tracking whether
a non-nullable local (from the function references proposal)
is initialized or not. This has the same problem as unreachable
code.
Attachment #9327148 - Attachment description: Bug 1823379 - wasm: Handle repeated catch clauses correctly. r?jseward → Bug 1823379. r=jseward

Comment on attachment 9327148 [details]
Bug 1823379. r=jseward

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Not easily. The patch indicates a change in the validation algorithm, but it's not obvious what the combination of instructions are required to trip it up. It's also not obvious after that how to cause a miscompilation (see previous comment on attempting to do this myself).
  • 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 supported
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: Yes
  • If not, how different, hard to create, and risky will they be?:
  • How likely is this patch to cause regressions; how much testing does it need?: Unlikely, the patch fixes a case where we should be rejecting code and other browsers would reject this code. Normal code on the web won't hit this. This also passes the test suite we have.
  • Is Android affected?: Yes
Attachment #9327148 - Flags: sec-approval?
Flags: sec-bounty?

Comment on attachment 9327148 [details]
Bug 1823379. r=jseward

Approved to land and uplift

Attachment #9327148 - Flags: sec-approval? → sec-approval+
Whiteboard: [reminder-test 2023-06-20]

r=jseward
https://hg.mozilla.org/integration/autoland/rev/3e6606109e0cc82ccddd438dd7725b4774442e9a
https://hg.mozilla.org/mozilla-central/rev/3e6606109e0c

Group: javascript-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 1 year ago
status-firefox114: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 114 Branch

The patch landed in nightly and beta is affected.
:rhunt, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox113 to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(rhunt)

Comment on attachment 9327148 [details]
Bug 1823379. r=jseward

Beta/Release Uplift Approval Request

  • User impact if declined: Invalid wasm code can crash a content process, attackers may be able to find a way to create miscompiled baseline wasm functions.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • 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): Unlikely, the patch fixes a case where we should be rejecting code and other browsers would reject this code. Normal code on the web won't hit this. This also passes the test suite we have.
  • String changes made/needed:
  • Is Android affected?: Yes
Flags: needinfo?(rhunt)
Attachment #9327148 - Flags: approval-mozilla-beta?

Comment on attachment 9327148 [details]
Bug 1823379. r=jseward

Approved for 113.0b5.

Attachment #9327148 - Flags: approval-mozilla-esr102?
Attachment #9327148 - Flags: approval-mozilla-beta?
Attachment #9327148 - Flags: approval-mozilla-beta+

Comment on attachment 9327148 [details]
Bug 1823379. r=jseward

Approved for 102.11esr.

Attachment #9327148 - Flags: approval-mozilla-esr102? → approval-mozilla-esr102+

I updated the existing phabricator revision for ESR.

Flags: needinfo?(rhunt)
Flags: sec-bounty? → sec-bounty+
Keywords: regression
Regressed by: 1759217
Whiteboard: [reminder-test 2023-06-20] → [reminder-test 2023-06-20][adv-main113+]
Whiteboard: [reminder-test 2023-06-20][adv-main113+] → [reminder-test 2023-06-20][adv-main113+][adv-ESR102.11+]
Flags: qe-verify-
Attached file advisory.txt —
Alias: CVE-2023-32211

2 months ago, tjr placed a reminder on the bug using the whiteboard tag [reminder-test 2023-06-20] .

rhunt, please refer to the original comment to better understand the reason for the reminder.

Flags: needinfo?(rhunt)
Whiteboard: [reminder-test 2023-06-20][adv-main113+][adv-ESR102.11+] → [adv-main113+][adv-ESR102.11+]
Group: core-security-release
Flags: needinfo?(rhunt)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: