Skip to content

Commit 4433fca

Browse files
panvasxa
authored andcommitted
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> PR-URL: #63111 Backport-PR-URL: #63563 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent b5cf012 commit 4433fca

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,
@@ -927,6 +928,8 @@ function isKeyObject(obj) {
927928
// CryptoKey's hidden class pristine. The `getCryptoKey{Type,
928929
// Extractable,Algorithm,Usages,Handle}` helpers index into that
929930
// array and convert native enums/masks back to Web Crypto strings.
931+
// The internal algorithm object is stored as a null-prototype clone
932+
// so it cannot observe polluted Object.prototype properties.
930933
// The public `algorithm` getter caches a cloned dictionary and the
931934
// public `usages` getter caches a synthesized array (as Web Crypto
932935
// requires repeat reads to return the same object so a consumer's
@@ -944,9 +947,27 @@ const kSlotUsages = 7;
944947

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

@@ -971,8 +992,8 @@ const {
971992
return `CryptoKey ${inspect({
972993
type: getCryptoKeyType(this),
973994
extractable: getCryptoKeyExtractable(this),
974-
algorithm: getCryptoKeyAlgorithm(this),
975-
usages: getCryptoKeyUsages(this),
995+
algorithm: cloneAlgorithm(getCryptoKeyAlgorithm(this)),
996+
usages: ArrayPrototypeSlice(getCryptoKeyUsages(this), 0),
976997
}, opts)}`;
977998
}
978999

@@ -1008,6 +1029,12 @@ const {
10081029
class InternalCryptoKey extends NativeCryptoKey {
10091030
#slots;
10101031

1032+
constructor(handle, algorithm, usagesMask, extractable) {
1033+
if (algorithm !== undefined)
1034+
algorithm = cloneInternalAlgorithm(algorithm);
1035+
super(handle, algorithm, usagesMask, extractable);
1036+
}
1037+
10111038
static {
10121039
getSlots = (key) => {
10131040
if (!key || typeof key !== 'object')
@@ -1017,6 +1044,7 @@ const {
10171044
if (cached !== undefined) return cached;
10181045
}
10191046
const slots = nativeGetCryptoKeySlots(key);
1047+
slots[kSlotAlgorithm] = cloneInternalAlgorithm(slots[kSlotAlgorithm]);
10201048
key.#slots = slots;
10211049
return slots;
10221050
};

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)