d3de21e23b  Add bounds checks to platformImportSpki and platformImportPkcs8

The bug at a glance

High. platformImportSpki and platformImportPkcs8 parse attacker-controlled ASN.1 key blobs supplied to crypto.subtle.importKey. A crafted length byte drives the parse index past the buffer end, causing keyData.size() - index to underflow (size_t) and subvector(index) to read out of bounds, crashing the WebContent process and constituting a heap over-read. Fully web-reachable with no special privileges; six regression tests and an rdar back it.

bytesUsedToEncodedLength() can return up to 128 for a 0xFF DER length byte. The parser advances index by that amount plus one without re-checking index against keyData.size(), so a single oversized final length byte pushes index beyond the buffer, and the subsequent unsigned subtraction keyData.size() - index wraps to a huge value fed to subspan/subvector.

Root cause

CryptoKeyEC::platformImportSpki and platformImportPkcs8 hand-parse DER-encoded SubjectPublicKeyInfo and PKCS#8 PrivateKeyInfo structures byte by byte. At each step they read a tag, then a length via bytesUsedToEncodedLength(keyData[index]) which returns the number of bytes the DER length field occupies; for a long-form length prefix like 0xFF this returns 128. The parser then advances index by that count (plus one for the tag/initial octet).

The pre-patch code performs a bounds check before reading the final length byte (if (keyData.size() < index + 1) return nullptr;), but performs no check after advancing index by bytesUsedToEncodedLength(…) + 1. Consequently, after the advance, index can point past the end of keyData. In platformImportSpki the code then computes keyData.size() - index and passes it to doesUncompressedPointMatchNamedCurve and keyData.subspan(index, keyData.size() - index). Because size() and index are unsigned (size_t), size() - index underflows to a near-SIZE_MAX value when index > size(), and subspan is asked to cover an enormous region starting past the buffer, an out-of-bounds read that crashes the process. In platformImportPkcs8 the analogous keyData.subvector(index) is called with index beyond the buffer, again reading out of bounds.

The fix inserts, in both functions, a check immediately after the final index advance: if (keyData.size() < index) return nullptr;. This rejects any blob whose computed index has run past the buffer before any subtraction or subvector call occurs, converting the crash into a graceful DataError promise rejection. Note the check is < (not <= or < index + 1) because index == keyData.size() is legitimately the empty-remainder case handled downstream (the empty-key-data tests confirm that index == size() must also reject cleanly at later validation, but does not itself overread).

This is explicitly the same class of bug previously fixed for RSA import in 308706@main; here it is closed for the EC importers. The attack surface is the standard Web Crypto importKey(‘spki’|‘pkcs8’, …) entry, so any origin can reach it.

Key code

Post-advance bounds guards added to both EC importers (CryptoKeyECCocoa.cpp)

     if (keyData.size() < index + 1)
         return nullptr;
     index += bytesUsedToEncodedLength(keyData[index]) + 1; // Read length
+    if (keyData.size() < index)
+        return nullptr;
     if (doesUncompressedPointMatchNamedCurve(curve, keyData.size() - index))
         return platformImportRaw(identifier, curve, Vector<uint8_t>(keyData.subspan(index, keyData.size() - index)), extractable, usages);

     // platformImportPkcs8:
     index += bytesUsedToEncodedLength(keyData[index]) + 1; // Read length, InitialOctet
+    if (keyData.size() < index)
+        return nullptr;
     auto keyBinary = keyData.subvector(index);

Patch walkthrough

• Source/WebCore/crypto/cocoa/CryptoKeyECCocoa.cpp — In platformImportSpki, after ‘index += bytesUsedToEncodedLength(keyData[index]) + 1; // Read length’ a new guard ‘if (keyData.size() < index) return nullptr;’ is added before the doesUncompressedPointMatchNamedCurve / subspan(index, keyData.size() - index) computation, preventing the size()-index underflow and OOB subspan. In platformImportPkcs8 the identical guard is added after the ‘Read length, InitialOctet’ advance, before ‘auto keyBinary = keyData.subvector(index);’, preventing subvector from being called with an index beyond the buffer.
• LayoutTests/crypto/subtle/ec-import-* — Twelve new EC test files (empty-key-data, invalid-length, truncated-key for both spki and pkcs8, plus expected.txt) craft blobs whose final 0x00/0xFF/0x82 length byte drives index to == size(), far past size(0xFF -> +128), or slightly past (0x82 -> +3), asserting importKey rejects with DataError rather than crashing.
• LayoutTests/crypto/subtle/rsa-import-* — The existing RSA import tests are migrated from js-test-pre.js/js-test-post.js to js-test.js and their expected output relaxed from ‘rejected promise with DataError: …’ to ‘rejected promise.’, aligning harness/output with the new EC tests.

Background

DER length encoding — ASN.1 DER lengths use short form (one byte, value < 128) or long form (0x81..0xFF, high bit set) where the low 7 bits give the number of subsequent length bytes. A 0xFF byte indicates 127 following bytes.

bytesUsedToEncodedLength() — Helper returning how many bytes a DER length field occupies given its first byte; returns up to 128 for 0xFF, which the parser uses to advance its index.

size_t underflow — keyData.size() - index is unsigned arithmetic; when index exceeds size() the result wraps to a value near SIZE_MAX, which then drives an enormous subspan length.

SubjectPublicKeyInfo / PKCS#8 — spki carries a public key, pkcs8 a private key, both as nested DER SEQUENCEs with algorithm OIDs; WebKit hand-parses these before handing the raw point/scalar to CryptoKit.

Vulnerability window

1. Entry — Page calls crypto.subtle.importKey(‘spki’|‘pkcs8’, blob, {name:‘ECDSA’, namedCurve:‘P-256’}, …).
2. Parse — platformImportSpki/Pkcs8 walk the DER header, passing all intermediate size checks.
3. Oversized length — The final BIT STRING/OCTET STRING length byte is 0xFF (or 0x82), so bytesUsedToEncodedLength returns 128 (or 3).
4. Advance past end — index += 128 + 1 moves index far beyond keyData.size().
5. Overread — keyData.size() - index underflows and subspan/subvector reads out of bounds, crashing WebContent.
6. Fix — if (keyData.size() < index) return nullptr; rejects before the subtraction, yielding a DataError.

Proof of concept

Verbatim core of ec-import-spki-invalid-length.html. The 25-byte blob is a well-formed SPKI header up to a final 0xFF length byte; pre-patch, index = 24 + 128 + 1 = 153 and keyData.size() - index = 25 - 153 underflows, overreading. Sibling tests use 0x82 (index=28, small overshoot) and 0x00 (index==size, empty remainder). The test asserts a clean rejection instead of a crash.

description("Test importing a SPKI ECDSA key with invalid length byte causing buffer overread");

var extractable = true;

// 25 bytes: valid SPKI header with IdEcPublicKey + Secp256r1 OIDs, then 0xFF length byte
var invalidLengthSpkiKey = hexStringToUint8Array("3000300006072a8648ce3d020106082a8648ce3d03010703ff");

// Should reject with DataError, not crash or read beyond buffer
shouldReject('crypto.subtle.importKey("spki", invalidLengthSpkiKey, {name: "ECDSA", namedCurve: "P-256"}, extractable, ["verify"])');

Exploitation

1. Reach — Any origin can call importKey with an attacker-crafted ArrayBuffer; no user gesture or permission needed.
2. Overread shape — By choosing the final length byte the attacker sets how far past the buffer index lands (a few bytes for 0x82, ~127 for 0xFF), and the underflow makes subspan cover a huge region.
3. Impact — The primary observable impact is an out-of-bounds read that crashes WebContent (DoS); combined with heap grooming it could serve as an info-leak read of adjacent heap during point parsing.
4. Constraint — The read result feeds doesUncompressedPointMatchNamedCurve/platformImportRaw, so directly exfiltrating leaked bytes is indirect; reliability of anything beyond a crash is limited.

Detection & hunting

For defenders and SOC / detection engineers:

• WebContent crash during importKey —
• Malformed DER length bytes —

Audit directions

• All hand-written DER/ASN.1 parsers —
• Unsigned subtraction on parse offsets —
• importKey format handlers —