Security: Potential Memory Safety Issue in ArrayBuffer JSI Converter

[tomaioo]

Summary

Security: Potential Memory Safety Issue in ArrayBuffer JSI Converter

Problem

Severity: High | File: packages/webgpu/cpp/rnwgpu/ArrayBuffer.h:L65

In ArrayBuffer.h, the createArrayBufferFromJSI function creates an shared_ptr that wraps raw pointers obtained from JSI ArrayBuffer objects. The byteOffset and byteLength values are cast from asNumber() without bounds checking against the actual ArrayBuffer size. A malicious or malformed JSI object could provide values that cause out-of-bounds memory access. Additionally, the bytesPerElements value is used without validation, which could lead to incorrect memory calculations.

Solution

Add strict bounds checking to ensure byteOffset + byteLength does not exceed the underlying ArrayBuffer size. Validate bytesPerElements is a positive integer within expected ranges. Consider using span or similar bounded view types instead of raw pointers.

Changes

• packages/webgpu/cpp/rnwgpu/ArrayBuffer.h (modified)

[wcandillon]

Would it be possible to had a test there?

[wcandillon]

if we guard here, should we also guard that this is an integer value?

[tomaioo]

Yes, we should validate that byteOffset, byteLength, and bytesPerElement are all integer values before the bounds check. Non-integer values from JSI could cause truncation issues or unexpected behavior in memory calculations. I'll add std::isfinite() and std::floor() == value checks, or use JSI's integer conversion if available, alongside the bounds validation.

[tomaioo]

Yes, we should validate that byteOffset, byteLength, and bytesPerElement are all integer values before the bounds check. Non-integer values from JSI could cause truncation issues or unexpected behavior in memory calculations. I'll add std::isfinite() and std::floor() == value checks, or use JSI's integer conversion if available, alongside the bounds validation.

[tomaioo]

Yes, we should validate that byteOffset, byteLength, and bytesPerElement are all integer values before the bounds check. Non-integer values from JSI could cause truncation issues or unexpected behavior in memory calculations. I'll add std::isfinite() and std::floor() == value checks, or use JSI's integer conversion if available, alongside the bounds validation.

[tomaioo]

Yes, we should validate that byteOffset, byteLength, and bytesPerElement are all integer values before the bounds check. Non-integer values from JSI could cause truncation issues or unexpected behavior in memory calculations. I'll add std::isfinite() and std::floor() == value checks, or use JSI's integer conversion if available, alongside the bounds validation.

[tomaioo]

Yes, we should validate that byteOffset, byteLength, and bytesPerElement are all integer values before the bounds check. Non-integer values from JSI could cause truncation issues or unexpected behavior in memory calculations. I'll add std::isfinite() and std::floor() == value checks, or use JSI's integer conversion if available, alongside the bounds validation.

[tomaioo]

Yes, we should validate that byteOffset, byteLength, and bytesPerElement are all integer values before the bounds check. Non-integer values from JSI could cause truncation issues or unexpected behavior in memory calculations. I'll add std::isfinite() and std::floor() == value checks, or use JSI's integer conversion if available, alongside the bounds validation.

[tomaioo]

Yes, we should validate that byteOffset, byteLength, and bytesPerElement are all integer values before the bounds check. Non-integer values from JSI could cause truncation issues or unexpected behavior in memory calculations. I'll add std::isfinite() and std::floor() == value checks, or use JSI's integer conversion if available, alongside the bounds validation.