GeezerVison is showing github.com — crypto: harden CryptoKey algorithm slots · nodejs/node@fd509a7
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/crypto/keys.js +32-4Lines changed: 32 additions & 4 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 3 | 3 | const { | 4 | 4 | ArrayPrototypeSlice, | 5 | 5 | ObjectDefineProperties, | | 6 | + ObjectPrototypeHasOwnProperty, | 6 | 7 | ObjectSetPrototypeOf, | 7 | 8 | SafeSet, | 8 | 9 | SymbolToStringTag, | @@ -932,6 +933,8 @@ function isKeyObject(obj) { | 932 | 933 | // CryptoKey's hidden class pristine. The `getCryptoKey{Type, | 933 | 934 | // Extractable,Algorithm,Usages,Handle}` helpers index into that | 934 | 935 | // 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. | 935 | 938 | // The public `algorithm` getter caches a cloned dictionary and the | 936 | 939 | // public `usages` getter caches a synthesized array (as Web Crypto | 937 | 940 | // requires repeat reads to return the same object so a consumer's | @@ -949,9 +952,27 @@ const kSlotUsages = 7; | 949 | 952 |
| 950 | 953 | function cloneAlgorithm(raw) { | 951 | 954 | const cloned = { ...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 = new Uint8Array(cloned.publicExponent); | | 962 | + } | | 963 | + return cloned; | | 964 | +} | | 965 | + | | 966 | +function cloneInternalAlgorithm(raw) { | | 967 | + const cloned = { __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) { | 954 | 974 | cloned.publicExponent = new Uint8Array(cloned.publicExponent); | | 975 | + } | 955 | 976 | return cloned; | 956 | 977 | } | 957 | 978 |
| @@ -976,8 +997,8 @@ const { | 976 | 997 | return `CryptoKey ${inspect({ | 977 | 998 | type: getCryptoKeyType(this), | 978 | 999 | extractable: getCryptoKeyExtractable(this), | 979 | | - algorithm: getCryptoKeyAlgorithm(this), | 980 | | - usages: getCryptoKeyUsages(this), | | 1000 | + algorithm: cloneAlgorithm(getCryptoKeyAlgorithm(this)), | | 1001 | + usages: ArrayPrototypeSlice(getCryptoKeyUsages(this), 0), | 981 | 1002 | }, opts)}`; | 982 | 1003 | } | 983 | 1004 |
| @@ -1013,6 +1034,12 @@ const { | 1013 | 1034 | class InternalCryptoKey extends NativeCryptoKey { | 1014 | 1035 | #slots; | 1015 | 1036 |
| | 1037 | + constructor(handle, algorithm, usagesMask, extractable) { | | 1038 | + if (algorithm !== undefined) | | 1039 | + algorithm = cloneInternalAlgorithm(algorithm); | | 1040 | + super(handle, algorithm, usagesMask, extractable); | | 1041 | + } | | 1042 | + | 1016 | 1043 | static { | 1017 | 1044 | getSlots = (key) => { | 1018 | 1045 | if (!key || typeof key !== 'object') | @@ -1022,6 +1049,7 @@ const { | 1022 | 1049 | if (cached !== undefined) return cached; | 1023 | 1050 | } | 1024 | 1051 | const slots = nativeGetCryptoKeySlots(key); | | 1052 | + slots[kSlotAlgorithm] = cloneInternalAlgorithm(slots[kSlotAlgorithm]); | 1025 | 1053 | key.#slots = slots; | 1026 | 1054 | return slots; | 1027 | 1055 | }; | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-webcrypto-cryptokey-hidden-slots.js +65Lines changed: 65 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -47,6 +47,71 @@ common.expectWarning({ | 47 | 47 | false, | 48 | 48 | ['sign', 'verify'], | 49 | 49 | ); | | 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 | + } | 50 | 115 |
| 51 | 116 | // Snapshot the real values BEFORE tampering. | 52 | 117 | const realType = key.type; | |
|