From 91f6a27de3539f649c84766facb4e2a2331feb06 Mon Sep 17 00:00:00 2001 From: Jeremy Banks <_@jeremy.ca> Date: Sun, 12 Nov 2023 20:49:07 -0500 Subject: [PATCH 1/4] fix(crypto): correct invalid test logic that was supressing some failures --- crypto/crypto_test.ts | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/crypto/crypto_test.ts b/crypto/crypto_test.ts index cb08a04e7306..5366c53015ca 100644 --- a/crypto/crypto_test.ts +++ b/crypto/crypto_test.ts @@ -1,5 +1,5 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. -import { assert, assertEquals, assertInstanceOf } from "../assert/mod.ts"; +import { assert, assertEquals, assertInstanceOf, fail } from "../assert/mod.ts"; import { crypto as stdCrypto } from "./mod.ts"; import { repeat } from "../bytes/repeat.ts"; import { dirname, fromFileUrl } from "../path/mod.ts"; @@ -1363,7 +1363,11 @@ for (const algorithm of digestAlgorithms) { const bytePieces = pieces.map((piece) => typeof piece === "string" ? new TextEncoder().encode(piece) : piece ) as Array; - try { + + // Expected value will either be a hex string, if the case is expected + // to return successfully, or an error class/constructor function, if + // the case is expected to throw. + if (typeof expected === "string") { const actual = toHexString( await stdCrypto.subtle.digest({ ...options, @@ -1379,15 +1383,32 @@ for (const algorithm of digestAlgorithms) { JSON.stringify(options) }) returned unexpected value\n actual: ${actual}\nexpected: ${expected}`, ); - } catch (error) { - if (expected instanceof Function) { - assert( - error instanceof expected, - `got a different error than expected: ${error}`, + } else if (typeof expected === "function") { + let error; + try { + await stdCrypto.subtle.digest({ + ...options, + name: algorithm, + }, bytePieces); + } catch (caughtError) { + error = caughtError; + } + if (error !== undefined) { + assertInstanceOf( + error, + expected, ); } else { - throw error; + fail( + `${algorithm} of ${caption}${ + i > 0 ? ` (but not until variation [${i}]!)` : "" + } with options ${ + JSON.stringify(options) + }) expected an exception of type ${expected.name}, but none was thrown.`, + ); } + } else { + throw new TypeError("expected value has an unexpected type"); } } } From 37fabb63fb7bbc5b497a045530e899231f912aae Mon Sep 17 00:00:00 2001 From: Jeremy Banks <_@jeremy.ca> Date: Mon, 13 Nov 2023 11:07:52 -0500 Subject: [PATCH 2/4] fix(crypto): validate that digest length must be an integer between 0 and isize::MAX --- crypto/crypto.ts | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/crypto/crypto.ts b/crypto/crypto.ts index 8a65ee83cbd6..16e9bb4d9e8c 100644 --- a/crypto/crypto.ts +++ b/crypto/crypto.ts @@ -220,6 +220,17 @@ const stdCrypto: StdCrypto = ((x) => x)({ data: BufferSource | AsyncIterable | Iterable, ): Promise { const { name, length } = normalizeAlgorithm(algorithm); + + if ( + length !== undefined && + (length < 0 || length > maximumDigestLength || + !Number.isInteger(length)) + ) { + throw new RangeError( + `length must be an integer between 0 and ${maximumDigestLength}, inclusive`, + ); + } + const bytes = bufferSourceBytes(data); if (FNVAlgorithms.includes(name)) { @@ -280,20 +291,30 @@ const stdCrypto: StdCrypto = ((x) => x)({ algorithm: DigestAlgorithm, data: BufferSource | Iterable, ): ArrayBuffer { - algorithm = normalizeAlgorithm(algorithm); + const { name, length } = normalizeAlgorithm(algorithm); + + if ( + length !== undefined && + (length < 0 || length > maximumDigestLength || + !Number.isInteger(length)) + ) { + throw new RangeError( + `length must be an integer between 0 and ${maximumDigestLength}, inclusive`, + ); + } const bytes = bufferSourceBytes(data); - if (FNVAlgorithms.includes(algorithm.name)) { - return fnv(algorithm.name, bytes); + if (FNVAlgorithms.includes(name)) { + return fnv(name, bytes); } const wasmCrypto = instantiateWasm(); if (bytes) { - return wasmCrypto.digest(algorithm.name, bytes, algorithm.length) + return wasmCrypto.digest(name, bytes, length) .buffer; } else if ((data as Iterable)[Symbol.iterator]) { - const context = new wasmCrypto.DigestContext(algorithm.name); + const context = new wasmCrypto.DigestContext(name); for (const chunk of data as Iterable) { const chunkBytes = bufferSourceBytes(chunk); if (!chunkBytes) { @@ -301,7 +322,7 @@ const stdCrypto: StdCrypto = ((x) => x)({ } context.update(chunkBytes); } - return context.digestAndDrop(algorithm.length).buffer; + return context.digestAndDrop(length).buffer; } else { throw new TypeError( "data must be a BufferSource or Iterable", @@ -328,6 +349,13 @@ const webCryptoDigestAlgorithms = [ export type FNVAlgorithms = "FNV32" | "FNV32A" | "FNV64" | "FNV64A"; export type DigestAlgorithmName = WasmDigestAlgorithm | FNVAlgorithms; +/* + * The largest digest length the current WASM implementation can support. This + * is the value of `isize::MAX` on 32-bit platforms like WASM, which is the + * maximum allowed capacity of a Rust `Vec`. + */ +const maximumDigestLength = 0x7FFF_FFFF; + export type DigestAlgorithmObject = { name: DigestAlgorithmName; length?: number; From f227a2d933f0c55d51b6d2dfbfbd9e3e3e113f30 Mon Sep 17 00:00:00 2001 From: Jeremy Banks <_@jeremy.ca> Date: Tue, 14 Nov 2023 10:45:46 -0500 Subject: [PATCH 3/4] style(crypto): minor tweaks in response to review --- crypto/crypto.ts | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/crypto/crypto.ts b/crypto/crypto.ts index 16e9bb4d9e8c..61fd4175402f 100644 --- a/crypto/crypto.ts +++ b/crypto/crypto.ts @@ -221,15 +221,7 @@ const stdCrypto: StdCrypto = ((x) => x)({ ): Promise { const { name, length } = normalizeAlgorithm(algorithm); - if ( - length !== undefined && - (length < 0 || length > maximumDigestLength || - !Number.isInteger(length)) - ) { - throw new RangeError( - `length must be an integer between 0 and ${maximumDigestLength}, inclusive`, - ); - } + assertValidDigestLength(length); const bytes = bufferSourceBytes(data); @@ -293,15 +285,7 @@ const stdCrypto: StdCrypto = ((x) => x)({ ): ArrayBuffer { const { name, length } = normalizeAlgorithm(algorithm); - if ( - length !== undefined && - (length < 0 || length > maximumDigestLength || - !Number.isInteger(length)) - ) { - throw new RangeError( - `length must be an integer between 0 and ${maximumDigestLength}, inclusive`, - ); - } + assertValidDigestLength(length); const bytes = bufferSourceBytes(data); @@ -352,9 +336,25 @@ export type DigestAlgorithmName = WasmDigestAlgorithm | FNVAlgorithms; /* * The largest digest length the current WASM implementation can support. This * is the value of `isize::MAX` on 32-bit platforms like WASM, which is the - * maximum allowed capacity of a Rust `Vec`. + * maximum allowed capacity of a Rust `Vec`. + */ +const MAX_DIGEST_LENGTH = 0x7FFF_FFFF; + +/** + * Asserts that a number is a valid length for a digest, which must be an + * integer that fits in a Rust `Vec`, or be undefined. */ -const maximumDigestLength = 0x7FFF_FFFF; +const assertValidDigestLength = (value?: number) => { + if ( + value !== undefined && + (value < 0 || value > MAX_DIGEST_LENGTH || + !Number.isInteger(value)) + ) { + throw new RangeError( + `length must be an integer between 0 and ${MAX_DIGEST_LENGTH}, inclusive`, + ); + } +}; export type DigestAlgorithmObject = { name: DigestAlgorithmName; From 9d62baa55e3c3520af05a2561d1b5eae3c19c11d Mon Sep 17 00:00:00 2001 From: Asher Gomez Date: Wed, 15 Nov 2023 10:56:43 +1100 Subject: [PATCH 4/4] Update crypto/crypto.ts --- crypto/crypto.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/crypto.ts b/crypto/crypto.ts index 61fd4175402f..1ac5da6a7c6d 100644 --- a/crypto/crypto.ts +++ b/crypto/crypto.ts @@ -344,7 +344,7 @@ const MAX_DIGEST_LENGTH = 0x7FFF_FFFF; * Asserts that a number is a valid length for a digest, which must be an * integer that fits in a Rust `Vec`, or be undefined. */ -const assertValidDigestLength = (value?: number) => { +function assertValidDigestLength(value?: number) { if ( value !== undefined && (value < 0 || value > MAX_DIGEST_LENGTH || @@ -354,7 +354,7 @@ const assertValidDigestLength = (value?: number) => { `length must be an integer between 0 and ${MAX_DIGEST_LENGTH}, inclusive`, ); } -}; +} export type DigestAlgorithmObject = { name: DigestAlgorithmName;