Skip to content

Commit b4dd729

Browse files
committed
crypto: harden CryptoKey algorithm slots
Clone CryptoKey algorithm dictionaries into null-prototype objects before storing or caching them internally. Copy nested hash dictionaries and publicExponent bytes so internal consumers and transferred keys do not observe user-mutable input objects or polluted Object.prototype fields. Keep public algorithm and inspect output as ordinary objects. Make the clone path check only own hash and publicExponent properties. Signed-off-by: Filip Skokan <panva.ip@gmail.com>
1 parent 28920ad commit b4dd729

2 files changed

Lines changed: 97 additions & 4 deletions

File tree

lib/internal/crypto/keys.js

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
const {
44
ArrayPrototypeSlice,
55
ObjectDefineProperties,
6+
ObjectPrototypeHasOwnProperty,
67
ObjectSetPrototypeOf,
78
SafeSet,
89
SymbolToStringTag,
@@ -928,6 +929,8 @@ function isKeyObject(obj) {
928929
// CryptoKey's hidden class pristine. The `getCryptoKey{Type,
929930
// Extractable,Algorithm,Usages,Handle}` helpers index into that
930931
// array and convert native enums/masks back to Web Crypto strings.
932+
// The internal algorithm object is stored as a null-prototype clone
933+
// so it cannot observe polluted Object.prototype properties.
931934
// The public `algorithm` getter caches a cloned dictionary and the
932935
// public `usages` getter caches a synthesized array (as Web Crypto
933936
// requires repeat reads to return the same object so a consumer's
@@ -945,9 +948,27 @@ const kSlotUsages = 7;
945948

946949
function cloneAlgorithm(raw) {
947950
const cloned = { ...raw };
948-
if (cloned.hash !== undefined) cloned.hash = { ...cloned.hash };
949-
if (cloned.publicExponent !== undefined)
951+
if (ObjectPrototypeHasOwnProperty(cloned, 'hash') &&
952+
cloned.hash !== undefined) {
953+
cloned.hash = { ...cloned.hash };
954+
}
955+
if (ObjectPrototypeHasOwnProperty(cloned, 'publicExponent') &&
956+
cloned.publicExponent !== undefined) {
957+
cloned.publicExponent = new Uint8Array(cloned.publicExponent);
958+
}
959+
return cloned;
960+
}
961+
962+
function cloneInternalAlgorithm(raw) {
963+
const cloned = { __proto__: null, ...raw };
964+
if (ObjectPrototypeHasOwnProperty(cloned, 'hash') &&
965+
cloned.hash !== undefined) {
966+
cloned.hash = { __proto__: null, ...cloned.hash };
967+
}
968+
if (ObjectPrototypeHasOwnProperty(cloned, 'publicExponent') &&
969+
cloned.publicExponent !== undefined) {
950970
cloned.publicExponent = new Uint8Array(cloned.publicExponent);
971+
}
951972
return cloned;
952973
}
953974

@@ -972,8 +993,8 @@ const {
972993
return `CryptoKey ${inspect({
973994
type: getCryptoKeyType(this),
974995
extractable: getCryptoKeyExtractable(this),
975-
algorithm: getCryptoKeyAlgorithm(this),
976-
usages: getCryptoKeyUsages(this),
996+
algorithm: cloneAlgorithm(getCryptoKeyAlgorithm(this)),
997+
usages: ArrayPrototypeSlice(getCryptoKeyUsages(this), 0),
977998
}, opts)}`;
978999
}
9791000

@@ -1009,6 +1030,12 @@ const {
10091030
class InternalCryptoKey extends NativeCryptoKey {
10101031
#slots;
10111032

1033+
constructor(handle, algorithm, usagesMask, extractable) {
1034+
if (algorithm !== undefined)
1035+
algorithm = cloneInternalAlgorithm(algorithm);
1036+
super(handle, algorithm, usagesMask, extractable);
1037+
}
1038+
10121039
static {
10131040
getSlots = (key) => {
10141041
if (!key || typeof key !== 'object')
@@ -1018,6 +1045,7 @@ const {
10181045
if (cached !== undefined) return cached;
10191046
}
10201047
const slots = nativeGetCryptoKeySlots(key);
1048+
slots[kSlotAlgorithm] = cloneInternalAlgorithm(slots[kSlotAlgorithm]);
10211049
key.#slots = slots;
10221050
return slots;
10231051
};

test/parallel/test-webcrypto-cryptokey-hidden-slots.js

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,71 @@ common.expectWarning({
4747
false,
4848
['sign', 'verify'],
4949
);
50+
const { publicKey: rsaPublicKey } = await subtle.generateKey(
51+
{
52+
name: 'RSA-PSS',
53+
modulusLength: 1024,
54+
publicExponent: new Uint8Array([1, 0, 1]),
55+
hash: 'SHA-256',
56+
},
57+
true,
58+
['sign', 'verify'],
59+
);
60+
61+
// Public algorithm/usages objects are mutable, but they must be
62+
// separate from the native-backed internal slots.
63+
rsaPublicKey.algorithm.name = 'FORGED-ALGORITHM';
64+
rsaPublicKey.algorithm.hash.name = 'FORGED-HASH';
65+
rsaPublicKey.algorithm.publicExponent[0] = 0xff;
66+
rsaPublicKey.usages.push('forged-usage');
67+
68+
const clonedRsaPublicKey = structuredClone(rsaPublicKey);
69+
assert.strictEqual(clonedRsaPublicKey.algorithm.name, 'RSA-PSS');
70+
assert.strictEqual(clonedRsaPublicKey.algorithm.hash.name, 'SHA-256');
71+
assert.deepStrictEqual(
72+
clonedRsaPublicKey.algorithm.publicExponent,
73+
new Uint8Array([1, 0, 1]));
74+
assert.deepStrictEqual(clonedRsaPublicKey.usages, ['verify']);
75+
76+
const rsaJwk = await subtle.exportKey('jwk', rsaPublicKey);
77+
assert.strictEqual(rsaJwk.alg, 'PS256');
78+
assert.deepStrictEqual(rsaJwk.key_ops, ['verify']);
79+
80+
Object.defineProperties(Object.prototype, {
81+
hash: {
82+
configurable: true,
83+
value: { name: 'FORGED-HASH' },
84+
},
85+
publicExponent: {
86+
configurable: true,
87+
value: new Uint8Array([0xff]),
88+
},
89+
});
90+
91+
try {
92+
const aesKey = await subtle.generateKey(
93+
{ name: 'AES-GCM', length: 128 },
94+
true,
95+
['encrypt'],
96+
);
97+
assert.deepStrictEqual(aesKey.algorithm, {
98+
name: 'AES-GCM',
99+
length: 128,
100+
});
101+
assert.strictEqual(Object.hasOwn(aesKey.algorithm, 'hash'), false);
102+
assert.strictEqual(
103+
Object.hasOwn(aesKey.algorithm, 'publicExponent'),
104+
false);
105+
106+
const clonedAesKey = structuredClone(aesKey);
107+
assert.deepStrictEqual(clonedAesKey.algorithm, {
108+
name: 'AES-GCM',
109+
length: 128,
110+
});
111+
} finally {
112+
delete Object.prototype.hash;
113+
delete Object.prototype.publicExponent;
114+
}
50115

51116
// Snapshot the real values BEFORE tampering.
52117
const realType = key.type;

0 commit comments

Comments
 (0)