Skip to content

Commit fd509a7

Browse files
panvaaduh95
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 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent 8657df3 commit fd509a7

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,
@@ -932,6 +933,8 @@ function isKeyObject(obj) {
932933
// CryptoKey's hidden class pristine. The `getCryptoKey{Type,
933934
// Extractable,Algorithm,Usages,Handle}` helpers index into that
934935
// array and convert native enums/masks back to Web Crypto strings.
936+
// The internal algorithm object is stored as a null-prototype clone
937+
// so it cannot observe polluted Object.prototype properties.
935938
// The public `algorithm` getter caches a cloned dictionary and the
936939
// public `usages` getter caches a synthesized array (as Web Crypto
937940
// requires repeat reads to return the same object so a consumer's
@@ -949,9 +952,27 @@ const kSlotUsages = 7;
949952

950953
functioncloneAlgorithm(raw){
951954
constcloned={ ...raw};
952-
if(cloned.hash!==undefined)cloned.hash={ ...cloned.hash};
953-
if(cloned.publicExponent!==undefined)
955+
if(ObjectPrototypeHasOwnProperty(cloned,'hash')&&
956+
cloned.hash!==undefined){
957+
cloned.hash={ ...cloned.hash};
958+
}
959+
if(ObjectPrototypeHasOwnProperty(cloned,'publicExponent')&&
960+
cloned.publicExponent!==undefined){
961+
cloned.publicExponent=newUint8Array(cloned.publicExponent);
962+
}
963+
returncloned;
964+
}
965+
966+
functioncloneInternalAlgorithm(raw){
967+
constcloned={__proto__: null, ...raw};
968+
if(ObjectPrototypeHasOwnProperty(cloned,'hash')&&
969+
cloned.hash!==undefined){
970+
cloned.hash={__proto__: null, ...cloned.hash};
971+
}
972+
if(ObjectPrototypeHasOwnProperty(cloned,'publicExponent')&&
973+
cloned.publicExponent!==undefined){
954974
cloned.publicExponent=newUint8Array(cloned.publicExponent);
975+
}
955976
returncloned;
956977
}
957978

@@ -976,8 +997,8 @@ const {
976997
return`CryptoKey ${inspect({
977998
type: getCryptoKeyType(this),
978999
extractable: getCryptoKeyExtractable(this),
979-
algorithm: getCryptoKeyAlgorithm(this),
980-
usages: getCryptoKeyUsages(this),
1000+
algorithm: cloneAlgorithm(getCryptoKeyAlgorithm(this)),
1001+
usages: ArrayPrototypeSlice(getCryptoKeyUsages(this),0),
9811002
},opts)}`;
9821003
}
9831004

@@ -1013,6 +1034,12 @@ const {
10131034
classInternalCryptoKeyextendsNativeCryptoKey{
10141035
#slots;
10151036

1037+
constructor(handle,algorithm,usagesMask,extractable){
1038+
if(algorithm!==undefined)
1039+
algorithm=cloneInternalAlgorithm(algorithm);
1040+
super(handle,algorithm,usagesMask,extractable);
1041+
}
1042+
10161043
static{
10171044
getSlots=(key)=>{
10181045
if(!key||typeofkey!=='object')
@@ -1022,6 +1049,7 @@ const {
10221049
if(cached!==undefined)returncached;
10231050
}
10241051
constslots=nativeGetCryptoKeySlots(key);
1052+
slots[kSlotAlgorithm]=cloneInternalAlgorithm(slots[kSlotAlgorithm]);
10251053
key.#slots =slots;
10261054
returnslots;
10271055
};

‎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}=awaitsubtle.generateKey(
51+
{
52+
name: 'RSA-PSS',
53+
modulusLength: 1024,
54+
publicExponent: newUint8Array([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+
constclonedRsaPublicKey=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+
newUint8Array([1,0,1]));
74+
assert.deepStrictEqual(clonedRsaPublicKey.usages,['verify']);
75+
76+
constrsaJwk=awaitsubtle.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: newUint8Array([0xff]),
88+
},
89+
});
90+
91+
try{
92+
constaesKey=awaitsubtle.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+
constclonedAesKey=structuredClone(aesKey);
107+
assert.deepStrictEqual(clonedAesKey.algorithm,{
108+
name: 'AES-GCM',
109+
length: 128,
110+
});
111+
}finally{
112+
deleteObject.prototype.hash;
113+
deleteObject.prototype.publicExponent;
114+
}
50115

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

0 commit comments

Comments
 (0)