1756957 - (CVE-2022-28285) Incorrect AliasSet for MLoadTypedArrayElementHole

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P2)

Description

[lukas.bernhard]

Steps to reproduce:

The AliasSet of MLoadTypedArrayElementHole is currently set to AliasSet::Load(AliasSet::UnboxedElement). When lowered to either LLoadTypedArrayElementHole or LLoadTypedArrayElementHoleBigInt and eventually codegened, there is a masm.loadArrayBufferViewLengthIntPtr(...). Therefore, AliasSet::ArrayBufferViewLengthOrOffset should be added to the AliasSet. Furthermore, there is an access to ArrayBufferViewObject::dataOffset(). Therefore, AliasSet::ObjectFields should be added as well.

Comment 1

[Nicolas B. Pierron [:nbp]]

Sounds like the analysis in comment 0 is correct, from what I understand.
This should also be an easy patch to add it to MLoadTypedArrayElementHole::getAliasSet

Lukas, if you are interested in contributing a patch as well, to have a patch in firefox in addition to having found the issue ;)
Otherwise, any of us can take make the patch.

Comment 2

[lukas.bernhard]

Attachment: Bug 1756957 - Fix AliasSet of MLoadTypedArrayElementHole - r=nbp

Comment 3

[Nicolas B. Pierron [:nbp]]

Setting security rating to sec-moderate, as there is no AliasSet with Store(ArrayBufferViewLengthOrOffset). Not having any of this kind implies that any mutation to the length would have to be done through a call which has a Store(Any).

The other missing AliasSet with Store(ObjectFields), is about the manipulation of a reserved DATA_SLOT slot. The only MIR instruction which might manipulate typed arrays, would be a MMegamorphicStoreSlot, but reserved slot do not have names associated to them.

Thus, this bug does not sounds actionable on its own, but is definitely not correct and could be used with another issue as a way to do random memory read.

Comment 4

[lukas.bernhard]

Bug 829896 changed the AliasSet from AliasSet::Store(AliasSet::Any) to AliasSet::Load(AliasSet::TypedArrayElement), CodeGenerator.cpp already accessed dataOffset() at this point.

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Fix AliasSet of MLoadTypedArrayElementHole - r=nbp
https://hg.mozilla.org/integration/autoland/rev/d4ffb09f40d5b78e04e5ff6cafead2f0da27f594
https://hg.mozilla.org/mozilla-central/rev/d4ffb09f40d5

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Nice find, Lukas! Please request Beta and ESR91 approval on this when you get a chance.

Comment 7

[lukas.bernhard]

Comment on attachment 9265934 [details]
Bug 1756957 - Fix AliasSet of MLoadTypedArrayElementHole - r=nbp

Beta/Release Uplift Approval Request

• User impact if declined: In combination with another issue, this bug could serve as a stepping stone for arbitrary memory reads.
• 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): Unlikely to cause regressions, it just falls back on more conservative assumptions.
• String changes made/needed: N/A

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: In combination with another issue, this bug could serve as a stepping stone for arbitrary memory reads. It hence is more of a defense-in-depth request than directly affecting user security.
• User impact if declined: Weakening robustness of the the JS engine in the presence of another flaw.
• Fix Landed on Version: 100
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Unlikely to cause regressions, it just falls back on more conservative assumptions.

Comment 8

[Donal Meehan [:dmeehan]]

Comment on attachment 9265934 [details]
Bug 1756957 - Fix AliasSet of MLoadTypedArrayElementHole - r=nbp

Approved for 99.0b5. Thanks.

Comment 9

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9f48237af9fc

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9265934 [details]
Bug 1756957 - Fix AliasSet of MLoadTypedArrayElementHole - r=nbp

Approved for 91.8esr.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/29f793e0f64d

Comment 12

[Tom Ritter [:tjr]]

Attachment: advisory.txt