1927226 - Potential Out-of-Bounds Access When Creating Large ArrayBuffers

Status: RESOLVED INVALID

(Core :: JavaScript Engine: JIT, defect)

Description

[Raphael]

While testing patch D205375, issues were discovered in the function MResizableDataViewByteLength::computeRange:

Although the current check with if constexpr is still left, it limits the range value to INT32_MAX, which may not be sufficient if ByteLengthLimit is greater than INT32_MAX. Using if constexpr here is also unsafe, as it works at compile time rather than at runtime and cannot account for real-time size changes.

Potential Problems:

Memory Exhaustion: The browser allows the creation of ArrayBuffers larger than INT32_MAX, violating the allowable memory allocation limits. Creating a series of such arrays (up to 64 GB and more) leads to exhaustion of available memory and a browser crash.

Out-of-Bounds Read/Write: Despite the expected blocking of out-of-bounds operations, tests have shown the possibility of writing beyond the array and attempting to read using TypedArrays. While confirming the reading of data beyond the array was unsuccessful, this may indicate a partial interception of out-of-bounds operations by the browser.
To Reproduce:

Memory Exhaustion: Create 32 ArrayBuffer objects with a size of INT32_MAX + 100 bytes. The approximate amount of memory required to trigger a crash is more than 64 GB. An Out of Memory exception is expected.

Out-of-Bounds Read/Write: Create an ArrayBuffer with a size exceeding the allowable INT32_MAX limit and bind it to a TypedArray, such as Uint8Array. Write unique values to indices INT32_MAX + 1 and INT32_MAX + 2 and then read them to confirm. Conduct a range test, writing and reading values to indices INT32_MAX + 10 to INT32_MAX + 20.
Security Implications:

Attackers could potentially read or modify data that is outside the array, which could lead to the leakage of sensitive information.

I have provided two tests and a log with Asan that confirms the browser crash and memory shortage.

Comment 1

[Raphael]

Attachment: OOB Test.html

Comment 2

[Raphael]

Attachment: out of memory.html

Comment 3

[Raphael]

Attachment: ff_asan_log.1632.txt

Comment 4

[Andrew McCreight [:mccr8]]

D205375 is from bug 1886849, so I'm adding that to the "see also" field.

Comment 5

[Jan de Mooij [:jandem]]

(In reply to Raphael from comment #0)

While testing patch D205375, issues were discovered in the function MResizableDataViewByteLength::computeRange:

Although the current check with if constexpr is still left, it limits the range value to INT32_MAX, which may not be sufficient if ByteLengthLimit is greater than INT32_MAX. Using if constexpr here is also unsafe, as it works at compile time rather than at runtime and cannot account for real-time size changes.

There are two cases:

• 64-bit build: ByteLengthLimit will always be greater than INT32_MAX so MResizableDataViewByteLength::computeRange doesn't attempt to compute a range which is always fine.
• 32-bit build: ByteLengthLimit is INT32_MAX and MResizableDataViewByteLength::computeRange uses that knowledge, but we'll never see a typed array or a data view with a larger length because that would violate this ByteLengthLimit.

I think this bug is invalid and based on a misreading of the range analysis code, but I'll take a closer look on Monday.

Comment 6

[Jan de Mooij [:jandem]]

(In reply to Raphael from comment #0)

Using if constexpr here is also unsafe, as it works at compile time rather than at runtime and cannot account for real-time size changes.

Note that ArrayBufferObject::ByteLengthLimit is a compile-time constant so it can't change at runtime.

Comment 7

[Raphael]

Attachment: text.html

Comment 8

[Raphael]

(In reply to Jan de Mooij [:jandem] from comment #5)

(In reply to Raphael from comment #0)

While testing patch D205375, issues were discovered in the function MResizableDataViewByteLength::computeRange:

Although the current check with if constexpr is still left, it limits the range value to INT32_MAX, which may not be sufficient if ByteLengthLimit is greater than INT32_MAX. Using if constexpr here is also unsafe, as it works at compile time rather than at runtime and cannot account for real-time size changes.

There are two cases:

• 64-bit build: ByteLengthLimit will always be greater than INT32_MAX so MResizableDataViewByteLength::computeRange doesn't attempt to compute a range which is always fine.
• 32-bit build: ByteLengthLimit is INT32_MAX and MResizableDataViewByteLength::computeRange uses that knowledge, but we'll never see a typed array or a data view with a larger length because that would violate this ByteLengthLimit.

I think this bug is invalid and based on a misreading of the range analysis code, but I'll take a closer look on Monday.

The current logic, based on 64-bit compilation, does not prevent Out-of-Bounds (OOB) access. Tests have shown that an ArrayBuffer larger than INT32_MAX does not block access to memory outside the allowed range.
Test results demonstrate the ability to read and write beyond INT32_MAX. Although the ByteLengthLimit on 64-bit systems is indeed larger than INT32_MAX, it does not prevent OOB access. The actual match of data when reading and writing outside the range confirms a real vulnerability that the current patch implementation does not address.
Chrome: throws a RangeError: Array buffer allocation failed when trying to create an ArrayBuffer larger than INT32_MAX. This indicates an effective implementation of the array size limit, preventing OOB access.
Mozilla: allows creating an ArrayBuffer larger than INT32_MAX, and testing reveals the possibility of accessing beyond the array. Writing and reading data outside the allowed range is successful, indicating the presence of an OOB vulnerability.

Comment 9

[Jan de Mooij [:jandem]]

(In reply to Raphael from comment #8)

The current logic, based on 64-bit compilation, does not prevent Out-of-Bounds (OOB) access. Tests have shown that an ArrayBuffer larger than INT32_MAX does not block access to memory outside the allowed range.
Test results demonstrate the ability to read and write beyond INT32_MAX. Although the ByteLengthLimit on 64-bit systems is indeed larger than INT32_MAX, it does not prevent OOB access. The actual match of data when reading and writing outside the range confirms a real vulnerability that the current patch implementation does not address.

But why is writing to an index larger than INT32_MAX a problem? This is by design, see for example bug 1392234 and bug 1673557 where this was implemented.

Comment 10

[Raphael]

(In reply to Jan de Mooij [:jandem] from comment #9)

(In reply to Raphael from comment #8)

The current logic, based on 64-bit compilation, does not prevent Out-of-Bounds (OOB) access. Tests have shown that an ArrayBuffer larger than INT32_MAX does not block access to memory outside the allowed range.
Test results demonstrate the ability to read and write beyond INT32_MAX. Although the ByteLengthLimit on 64-bit systems is indeed larger than INT32_MAX, it does not prevent OOB access. The actual match of data when reading and writing outside the range confirms a real vulnerability that the current patch implementation does not address.

But why is writing to an index larger than INT32_MAX a problem? This is by design, see for example bug 1392234 and bug 1673557 where this was implemented.

Predictable and stable Out-of-Bounds (OOB) access: We are able to consistently read and write data beyond INT32_MAX, creating a potential Out-of-Bounds vulnerability. While this functionality may have been added intentionally, its predictability raises security concerns, particularly regarding the potential for data extraction or modification, especially if combined with other vulnerabilities.

Data integrity risks: The ability to write beyond array bounds could potentially alter data in neighboring memory regions. This creates a risk of data leakage or unintentional data modification if access to these memory regions occurs unexpectedly.

Audit for TypedArray/ArrayBuffer code: As you mentioned, supporting indices beyond INT32_MAX could benefit from a code audit across all length and offset operations. Predictable OOB access increases the risk of issues arising from implicit C++ type coercion, potentially leading to memory handling errors.

Comment 11

[Jan de Mooij [:jandem]]

(In reply to Raphael from comment #10)

The ability to write beyond array bounds could potentially alter data in neighboring memory regions.

Can you post a small test case where Firefox accesses data beyond the typed array or ArrayBuffer bounds? I agree that would be a security bug.

Comment 12

[Raphael]

I conducted the testing and here are the results I found.

Launching the extreme out-of-bounds ArrayBuffer access test...
Attempting to create an ArrayBuffer of size: 2147493647 bytes
Created ArrayBuffer of size: 2147493647 bytes
Length of Uint8Array: 2147493647
Extreme OOB Write succeeded: Set value 255 at index 2147483648
Extreme OOB Read: Read value 255 at index 2147483648
Extreme OOB Write succeeded: Set value 255 at index 2147484647
Extreme OOB Read: Read value 255 at index 2147484647
Extreme OOB Write succeeded: Set value 255 at index 2147488647
Extreme OOB Read: Read value 255 at index 2147488647
Extreme OOB Write succeeded: Set value 255 at index 2147493646
Extreme OOB Read: Read value 255 at index 2147493646
Extreme OOB Write succeeded: Set value 255 at index 2147494647
Extreme OOB Read: Read value undefined at index 2147494647
Extreme OOB Write succeeded: Set value 255 at index 2147593647
Extreme OOB Read: Read value undefined at index 2147593647

Writing and reading out-of-bounds of the array but within the buffer:

Indices: 2147483648, 2147484647, 2147488647, 2147493646

Result: Successful write and read of values (255), which violates the ECMAScript specification. According to the specification, writing beyond the array length should throw a RangeError.

Indices: 2147494647, 2147593647

Result: Write succeeded, but read returns undefined (partial compliance with the specification). Write should throw a RangeError.

According to the ECMAScript specification:

Reading out-of-bounds of the array (TypedArray): Should return undefined.

Writing out-of-bounds of the array (TypedArray): Should throw a RangeError exception.

The test example shows that in the 64-bit version of Firefox, there is a violation of the ECMAScript specification: attempts to write beyond the length of TypedArray do not throw a RangeError as prescribed. Although this does not provide access to arbitrary memory due to JavaScript isolation, such behavior is still a specification non-compliance and may affect the correctness of applications relying on strict TypedArray constraints.

This may not lead to a vulnerability but can lead to potential bugs

Comment 13

[Raphael]

Attachment: array test.html

Comment 14

[Jan de Mooij [:jandem]]

(In reply to Raphael from comment #12)

Writing and reading out-of-bounds of the array but within the buffer:

Indices: 2147483648, 2147484647, 2147488647, 2147493646

Result: Successful write and read of values (255), which violates the ECMAScript specification. According to the specification, writing beyond the array length should throw a RangeError.

The array buffer and typed array both have length 2147483647 + 10000 = 2147493647 and the indices you list are all less than that, so they aren't out of bounds.

Indices: 2147494647, 2147593647

Result: Write succeeded, but read returns undefined (partial compliance with the specification). Write should throw a RangeError.

According to the ECMAScript specification:

Reading out-of-bounds of the array (TypedArray): Should return undefined.

Writing out-of-bounds of the array (TypedArray): Should throw a RangeError exception.

This is incorrect. No exception should be thrown for out-of-bounds writes to typed arrays. See this part of the spec and the note at the end: https://tc39.es/ecma262/#sec-typedarraysetelement