PatchGap
Firefox · DOM

CVE-2024-10466

Logic Error in DOM
Low CVSS 7.5 firefox Logic Error 🔧 Commit mapped

Overview

Low
Severity
7.5
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactlow
DescriptionBy sending a specially crafted push message, a remote server could have hung the parent process, causing the browser to become unresponsive.
ComponentDOM
Bug ClassLogic Error
Tracker1924154
Fix commita76cdabc21ae (firefox) +18/-2
CWECWE-400
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CISA KEVNot listed
CreditedKagami Rosylight
Disclosed2024-10-29

Background

Web Push
a browser mechanism that lets a server deliver messages to a service worker via a push service, with the message payload encrypted end-to-end.
`aes128gcm` content encoding
the RFC 8188 encryption scheme whose binary header carries a salt, a record size (rs), an ID length, and a key ID, all parsed before decryption.
`rs` (record size)
the field controlling how large each encrypted record is, used to compute buffer sizes and the number of records to process.
`CryptoError`
the exception type PushCrypto.sys.mjs throws to reject a malformed or unsupported push payload before decryption proceeds.

Root Cause Analysis

The parent process decrypts incoming push payloads in PushCrypto.sys.mjs, parsing attacker-controlled header fields including the record size rs. In getCryptoParamsFromPayload, the 32-bit rs value read from the aes128gcm binary header was accepted without a lower-bound check, and in getCryptoParamsFromHeaders, the rs parsed from Crypto-Key/Encryption headers was only validated with isNaN, permitting 0 or 1. A record size below the minimum meaningful value of 18 (16-byte auth tag plus at least one byte of padding/content per RFC 8188) leaves no room for plaintext, so downstream record-counting arithmetic can iterate improperly or divide the payload into a degenerate/huge number of zero-progress records, hanging the parent process.

The fix rejects rs < 18 in getCryptoParamsFromPayload and constrains rs to 1 <= rs <= 68719476705 (2^36−31) in getCryptoParamsFromHeaders, throwing CryptoError with BAD_RS_PARAM so a bogus record size can never reach the decryption loop.

Key insight
The single core mistake was trusting the attacker-supplied record size rs without enforcing the RFC-mandated minimum, letting a degenerate value drive decryption arithmetic into an effectively unbounded loop; the fix adds explicit lower- and upper-bound validation on rs before any records are processed.

Attack Path

  1. Craft a malicious payload A remote server (or compromised push service) constructs an aes128gcm push message whose header encodes a record size rs below 18, such as 0 or 1.
  2. Deliver via push The message is delivered to the target browser’s push endpoint, which routes it into the parent process for decryption.
  3. Parse without bounds check getCryptoParamsFromPayload (or getCryptoParamsFromHeaders) reads the tiny rs value and, lacking a minimum-size guard, accepts it as valid crypto parameters.
  4. Enter degenerate decryption The undersized rs feeds record-count/buffer arithmetic that makes no forward progress or produces an enormous record count, spinning the parent process.
  5. Hang the browser The parent process becomes unresponsive, freezing the entire browser UI as a denial of service.

Impact Assessment

An attacker who can send a push message to a subscribed browser gains a denial-of-service primitive: they hang the privileged parent process, making the whole browser unresponsive. This executes in the parent-process push-decryption path and requires only that the victim have an active push subscription reachable by the attacker-controlled server, with no user interaction beyond the existing subscription.

Changed Functions

FunctionChangeNotes
if
dom/push/PushCrypto.sys.mjs
 modified
createHeader
dom/push/PushCrypto.sys.mjs
 modified

Files Changed

  • dom/push/PushCrypto.sys.mjs

Audit Directions

  • Unbounded attacker-controlled sizes
    Look for length, count, or size fields read from untrusted network payloads that feed loop bounds or buffer math without an explicit minimum/maximum check.
  • Spec-mandated invariants
    Audit binary format parsers (RFC 8188/8291 and similar) for missing enforcement of documented minimum field values, especially where a value of 0 or 1 is degenerate.
  • Parent-process decoders
    Review decryption and decoding paths that run in the privileged parent process, since a logic error there yields a full-browser DoS rather than a sandboxed crash.

Patch

view commit
diff --git a/dom/push/PushCrypto.sys.mjs b/dom/push/PushCrypto.sys.mjs
index fab1dc47629..78a6dc587ea 100644
--- a/dom/push/PushCrypto.sys.mjs
+++ b/dom/push/PushCrypto.sys.mjs
@@ -106,6 +106,8 @@ function getEncryptionParams(encryptField) {
 // aes128gcm scheme.
 function getCryptoParamsFromPayload(payload) {
   if (payload.byteLength < 21) {
+    // The value 21 is from https://datatracker.ietf.org/doc/html/rfc8188#section-2.1
+    // | salt (16) | rs (4) | idlen (1) | keyid (idlen) |
     throw new CryptoError("Truncated header", BAD_CRYPTO);
   }
   let rs =
@@ -113,8 +115,16 @@ function getCryptoParamsFromPayload(payload) {
     (payload[17] << 16) |
     (payload[18] << 8) |
     payload[19];
+  if (rs < 18) {
+    // https://datatracker.ietf.org/doc/html/rfc8188#section-2.1
+    throw new CryptoError(
+      "Record sizes smaller than 18 are invalid",
+      BAD_RS_PARAM
+    );
+  }
   let keyIdLen = payload[20];
   if (keyIdLen != 65) {
+    // https://datatracker.ietf.org/doc/html/rfc8291/#section-4
     throw new CryptoError("Invalid sender public key", BAD_DH_PARAM);
   }
   if (payload.byteLength <= 21 + keyIdLen) {
@@ -169,8 +179,12 @@ export function getCryptoParamsFromHeaders(headers) {
     throw new CryptoError("Invalid salt parameter", BAD_SALT_PARAM);
   }
   var rs = enc.rs ? parseInt(enc.rs, 10) : 4096;
-  if (isNaN(rs)) {
-    throw new CryptoError("rs parameter must be a number", BAD_RS_PARAM);
+  if (isNaN(rs) || rs < 1 || rs > 68719476705) {
+    // https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-encryption-encoding-03#section-3.1
+    throw new CryptoError(
+      "rs parameter must be a number greater than 1 and smaller than 2^36-31",
+      BAD_RS_PARAM
+    );
   }
   return {
     salt,
@@ -789,6 +803,7 @@ class aes128gcmEncoder {
   // Perform the actual encryption of the payload.
   async encrypt(key, nonce) {
     if (this.rs < 18) {
+      // https://datatracker.ietf.org/doc/html/rfc8188#section-2.1
       throw new CryptoError("recordsize is too small", BAD_RS_PARAM);
     }
 
@@ -867,6 +882,7 @@ class aes128gcmEncoder {
   createHeader(key) {
     // layout is "salt|32-bit-int|8-bit-int|key"
     if (key.byteLength != 65) {
+      // https://datatracker.ietf.org/doc/html/rfc8291/#section-4
       throw new CryptoError("Invalid key length for header", BAD_DH_PARAM);
     }
     // the 2 ints
Loading diff…
On This Page