Critical firefox Integer Overflow 🔧 Commit mapped

Overview

Critical
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactcritical
DescriptionMemory safety bugs present in Firefox ESR 115.35.0, Firefox ESR 140.10.0 and Firefox 150.0.0. Some of these bugs showed evidence of memory corruption and we presume that with enough effort some of these could have been exploited to run arbitrary code.
ComponentCore
Bug ClassInteger Overflow
Tracker2021904
Fix commit9968df8fbe02 (firefox) +61/-6
CISA KEVNot listed
CreditedC.M.Chang, Christian Holler, Steve Fink and the Mozilla Fuzzing Team
Disclosed2026-04-28

Background

`SlicedInputStream::Deserialize`
reconstructs a SlicedInputStream from SlicedInputStreamParams IPDL parameters (start, length, curPos) received over IPC from a child process; it validates the triple before trusting it.
`CheckedUint64`
SpiderMonkey/MFBT checked-arithmetic helper used in the old Deserialize path to detect overflow of start + length.
`kMaxStreamPos`
new constexpr uint64_t equal to INT64_MAX, introduced by the patch as the maximum legal internal offset so that start + length can never overflow int64_t-based stream APIs (e.g. Tell).

Root Cause Analysis

The constructor SlicedInputStream::SlicedInputStream previously stored the caller-supplied offsets verbatim: mStart(aStart) and mLength(aLength), with mStart and mLength both uint64_t. Nothing bounded their sum. Throughout the class, read bookkeeping computes positions as mStart + mLength (and compares against mCurPos), which can wrap modulo 2^64 when a caller passes a large aStart together with a large aLength (the added test StartMuchBiggerThan uses aStart = UINT64_MAX - 1, and LengthMuchBiggerThan uses aLength = UINT64_MAX - 1). A wrapped end offset makes the “have I reached the end of my slice?” arithmetic produce a value far smaller than either operand, so the stream can be driven to read past the intended slice boundary — the memory-corruption-class behavior the advisory presumes. The fix clamps both fields at construction time: mStart(std::clamp<uint64_t>(aStart, 0, kMaxStreamPos)) and mLength(std::clamp<uint64_t>(aLength, 0, kMaxStreamPos - mStart)). Because mStart ≤ INT64_MAX and mLength ≤ INT64_MAX - mStart, the sum mStart + mLength is now guaranteed ≤ INT64_MAX, i.e. it can never overflow even the signed 64-bit stream APIs.

The IPC entry point SlicedInputStream::Deserialize had an analogous but subtly broken guard. The old code computed auto end = CheckedUint64(params.start()) + params.length() and rejected the message only if !end.isValid() — but it then used end.value() in if (params.curPos() > end.value()), so on a genuine overflow it returned false (good), yet the structure of the check still relied on a single combined value. The patch replaces this with explicit per-field range checks against the new ceiling — if (params.start() > kMaxStreamPos || params.length() > kMaxStreamPos - params.start()) return false; — and then compares params.curPos() > params.start() + params.length() using values that are now proven not to overflow. This brings the deserialization validation in line with the constructor’s new invariant so a malicious child cannot smuggle an overflowing (start, length) pair across IPC.

The advisory labels this generically as “memory safety bugs … evidence of memory corruption” (bugClass: MemoryCorruption). The diff substantiates a more specific cause: an integer overflow of start + length offsets that was never range-checked at construction, which is why I classify it as IntegerOverflow.

Key insight
The constructor trusted two unbounded uint64_t range values and never ensured their sum stayed representable, so attacker-chosen start/length could wrap the end-of-slice arithmetic and let reads escape the intended slice; the fix clamps both to INT64_MAX so start + length can never overflow.

Attack Path

  1. Script/renderer Reach nsICloneableInputStreamWithRange::cloneWithRange(start, length) (or any API constructing a SlicedInputStream) with crafted uint64_t values such that start + length wraps past 2^64 (e.g. a near-UINT64_MAX start with a non-trivial length) (inferred: the diff proves the constructor is the sink and adds tests driving exactly these values).
  2. Boundary computation The unclamped mStart/mLength make the internal “end” (mStart + mLength) wrap to a small number, so slice-length and cursor comparisons no longer bound reads to the intended window.
  3. Alternative IPC vector A compromised child process sends SlicedInputStreamParams with an overflowing (start, length, curPos) triple; absent the new explicit range check the parent accepts bookkeeping that violates the slice invariant.
  4. Read past slice Driving Read/Available against the corrupted bounds consumes bytes beyond the intended sub-range of the underlying stream (out-of-slice / out-of-bounds read of stream contents).
  5. Primitive Out-of-bounds/overflow-driven read of stream data, which the fuzzing team observed as memory corruption; “with enough effort” potentially leveraged toward arbitrary code execution (the exploitability-to-RCE step is the advisory’s presumption, not proven by the diff).

Impact Assessment

The sink lives in XPCOM stream plumbing used by both renderer-reachable stream-cloning APIs and the IPC Deserialize path, so the overflow is reachable from a content process and, via SlicedInputStreamParams, from a compromised child over IPDL. No special privilege is required beyond the ability to construct a sliced stream with chosen start/length. The concrete primitive the diff supports is an integer-overflow-induced out-of-bounds read against stream contents; Mozilla’s advisory presumes memory corruption that could be escalated to code execution with effort, but the diff itself does not demonstrate a write primitive or full RCE — treat RCE as inferred. Gating: an attacker must supply range values whose sum overflows (very large start/length), which real callers never do but a malicious caller/child trivially can.

Files Changed

  • xpcom/io/SlicedInputStream.cpp
  • xpcom/io/SlicedInputStream.h
  • xpcom/io/nsICloneableInputStream.idl
  • xpcom/tests/gtest/TestSlicedInputStream.cpp

Audit Directions

  • Other `nsIInputStream` wrappers with offset+length math
    review sibling streams (e.g. partial/ranged stream wrappers, nsMultiplexInputStream, buffered/pipe wrappers) for places that store caller-supplied uint64_t start/length and later compute start + length or pos + count without a checked-arithmetic or clamp.
  • IPDL `Deserialize` validators
    audit other *Params deserializers that validate a combined value via CheckedUint64(...) + ... yet still use the combined value afterward; the pattern invites per-field vs combined-value mismatches like the one fixed here.
  • `cloneWithRange` / range-clone callers
    trace every caller that forwards web- or IPC-sourced start/length into stream constructors to confirm none assume non-overflowing inputs.
  • Signed/unsigned stream position APIs
    look for stream methods returning int64_t (e.g. Tell, Seek) fed by uint64_t internal offsets, where a value above INT64_MAX would reinterpret as negative.
diff --git a/xpcom/io/SlicedInputStream.cpp b/xpcom/io/SlicedInputStream.cpp
index 5a338e45d08..11da3a1395b 100644
--- a/xpcom/io/SlicedInputStream.cpp
+++ b/xpcom/io/SlicedInputStream.cpp
@@ -41,6 +41,15 @@ NS_INTERFACE_MAP_BEGIN(SlicedInputStream)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIInputStream)
 NS_INTERFACE_MAP_END
 
+// It is highly unlikely for a stream to exceed INT64_MAX bytes in length, so we
+// clamp these values here to allow callers as well as SlicedInputStream to
+// avoid unnecessary integer overflow checks. Out of range start/length values
+// should behave as expected in all realistic situations.
+//
+// Some stream APIs use an int64_t (e.g. Tell), so we use INT64_MAX as the
+// maximum internal offset for start/end.
+static constexpr uint64_t kMaxStreamPos = INT64_MAX;
+
 SlicedInputStream::SlicedInputStream(
     already_AddRefed<nsIInputStream> aInputStream, uint64_t aStart,
     uint64_t aLength)
@@ -51,8 +60,8 @@ SlicedInputStream::SlicedInputStream(
       mWeakAsyncInputStream(nullptr),
       mWeakInputStreamLength(nullptr),
       mWeakAsyncInputStreamLength(nullptr),
-      mStart(aStart),
-      mLength(aLength),
+      mStart(std::clamp<uint64_t>(aStart, 0, kMaxStreamPos)),
+      mLength(std::clamp<uint64_t>(aLength, 0, kMaxStreamPos - mStart)),
       mCurPos(0),
       mClosed(false),
       mAsyncWaitFlags(0),
@@ -489,11 +498,11 @@ bool SlicedInputStream::Deserialize(
 
   const SlicedInputStreamParams& params = aParams.get_SlicedInputStreamParams();
 
-  auto end = CheckedUint64(params.start()) + params.length();
-  if (!end.isValid()) {
+  if (params.start() > kMaxStreamPos ||
+      params.length() > kMaxStreamPos - params.start()) {
     return false;
   }
-  if (params.curPos() > end.value()) {
+  if (params.curPos() > params.start() + params.length()) {
     return false;
   }
 
diff --git a/xpcom/io/SlicedInputStream.h b/xpcom/io/SlicedInputStream.h
index c7a28d5ff3e..a266bbf5f7b 100644
--- a/xpcom/io/SlicedInputStream.h
+++ b/xpcom/io/SlicedInputStream.h
@@ -44,7 +44,11 @@ class SlicedInputStream final : public nsIAsyncInputStream,
   // than aStart bytes, reading from SlicedInputStream returns no data.  If
   // aInputStream contains more than aStart bytes, but fewer than aStart +
   // aLength bytes, reading from SlicedInputStream returns as many bytes as can
-  // be consumed from aInputStream after reading aLength bytes.
+  // be consumed from aInputStream after reading aStart bytes.
+  //
+  // It is safe to specify an arbitrarily large aLength (e.g. UINT64_MAX). Doing
+  // so is treated as allowing an arbitrary number of additional bytes following
+  // aStart.
   //
   // aInputStream should not be read from after constructing a
   // SlicedInputStream wrapper around it.
diff --git a/xpcom/io/nsICloneableInputStream.idl b/xpcom/io/nsICloneableInputStream.idl
index adefd0f4285..98790bd2d97 100644
--- a/xpcom/io/nsICloneableInputStream.idl
+++ b/xpcom/io/nsICloneableInputStream.idl
@@ -27,5 +27,12 @@ interface nsICloneableInputStream : nsISupports
 [scriptable, builtinclass, uuid(ece853c3-aded-4cef-8f51-0d1493d60bd5)]
 interface nsICloneableInputStreamWithRange : nsICloneableInputStream
 {
+  // Create a copy of the input stream, but with the data range reduced to a
+  // sub-slice. The copy will begin at `start` bytes, and extends for a maximum
+  // of `length` bytes. If the underlying stream contains more than `start`
+  // bytes, but fewer than `length` bytes, reading from the stream returns all
+  // remaining bytes.
+  //
+  // See SlicedInputStream's constructor for more details.
   nsIInputStream cloneWithRange(in uint64_t start, in uint64_t length);
 };
diff --git a/xpcom/tests/gtest/TestSlicedInputStream.cpp b/xpcom/tests/gtest/TestSlicedInputStream.cpp
index a2c1a077e48..ea9f606da09 100644
--- a/xpcom/tests/gtest/TestSlicedInputStream.cpp
+++ b/xpcom/tests/gtest/TestSlicedInputStream.cpp
@@ -307,6 +307,41 @@ TEST(TestSlicedInputStream, LengthBiggerThan)
   ASSERT_EQ((uint64_t)500, count);
 }
 
+// Like LengthBiggerThan, but for an overflowing aStart + aLength pair.
+TEST(TestSlicedInputStream, LengthMuchBiggerThan)
+{
+  nsCString buf;
+  RefPtr<SlicedInputStream> sis =
+      CreateNonSeekableStreams(500, 100, UINT64_MAX - 1, buf);
+
+  uint64_t length;
+  ASSERT_EQ(NS_OK, sis->Available(&length));
+  ASSERT_EQ((uint64_t)500 - 100, length);
+
+  char buf2[4096];
+  uint32_t count;
+  ASSERT_EQ(NS_OK, sis->Read(buf2, sizeof(buf2), &count));
+  ASSERT_EQ((uint64_t)(500 - 100), count);
+  ASSERT_EQ(Substring(buf, 100, count), Substring(buf2, count));
+}
+
+// Like LengthMuchBiggerThan, but with a massive aStart value.
+TEST(TestSlicedInputStream, StartMuchBiggerThan)
+{
+  nsCString buf;
+  RefPtr<SlicedInputStream> sis =
+      CreateNonSeekableStreams(500, UINT64_MAX - 1, 100, buf);
+
+  uint64_t length;
+  ASSERT_EQ(NS_OK, sis->Available(&length));
+  ASSERT_EQ((uint64_t)0, length);
+
+  char buf2[4096];
+  uint32_t count;
+  ASSERT_EQ(NS_OK, sis->Read(buf2, sizeof(buf2), &count));
+  ASSERT_EQ((uint64_t)0, count);
+}
+
 // What if the length is 0?
 TEST(TestSlicedInputStream, Length0)
 {
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/xpcom/tests/gtest/TestSlicedInputStream.cpp b/xpcom/tests/gtest/TestSlicedInputStream.cpp
index a2c1a077e48..ea9f606da09 100644
--- a/xpcom/tests/gtest/TestSlicedInputStream.cpp
+++ b/xpcom/tests/gtest/TestSlicedInputStream.cpp
@@ -307,6 +307,41 @@ TEST(TestSlicedInputStream, LengthBiggerThan)
   ASSERT_EQ((uint64_t)500, count);
 }
 
+// Like LengthBiggerThan, but for an overflowing aStart + aLength pair.
+TEST(TestSlicedInputStream, LengthMuchBiggerThan)
+{
+  nsCString buf;
+  RefPtr<SlicedInputStream> sis =
+      CreateNonSeekableStreams(500, 100, UINT64_MAX - 1, buf);
+
+  uint64_t length;
+  ASSERT_EQ(NS_OK, sis->Available(&length));
+  ASSERT_EQ((uint64_t)500 - 100, length);
+
+  char buf2[4096];
+  uint32_t count;
+  ASSERT_EQ(NS_OK, sis->Read(buf2, sizeof(buf2), &count));
+  ASSERT_EQ((uint64_t)(500 - 100), count);
+  ASSERT_EQ(Substring(buf, 100, count), Substring(buf2, count));
+}
+
+// Like LengthMuchBiggerThan, but with a massive aStart value.
+TEST(TestSlicedInputStream, StartMuchBiggerThan)
+{
+  nsCString buf;
+  RefPtr<SlicedInputStream> sis =
+      CreateNonSeekableStreams(500, UINT64_MAX - 1, 100, buf);
+
+  uint64_t length;
+  ASSERT_EQ(NS_OK, sis->Available(&length));
+  ASSERT_EQ((uint64_t)0, length);
+
+  char buf2[4096];
+  uint32_t count;
+  ASSERT_EQ(NS_OK, sis->Read(buf2, sizeof(buf2), &count));
+  ASSERT_EQ((uint64_t)0, count);
+}
+
 // What if the length is 0?
 TEST(TestSlicedInputStream, Length0)
 {
Loading diff…