GeezerVison is showing github.com — crypto: harden CryptoKey algorithm slots · nodejs/node@4433fca
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, | @@ -927,6 +928,8 @@ function isKeyObject(obj) { | 927 | 928 | // CryptoKey's hidden class pristine. The `getCryptoKey{Type, | 928 | 929 | // Extractable,Algorithm,Usages,Handle}` helpers index into that | 929 | 930 | // 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. | 930 | 933 | // The public `algorithm` getter caches a cloned dictionary and the | 931 | 934 | // public `usages` getter caches a synthesized array (as Web Crypto | 932 | 935 | // requires repeat reads to return the same object so a consumer's | @@ -944,9 +947,27 @@ const kSlotUsages = 7; | 944 | 947 |
| 945 | 948 | function cloneAlgorithm(raw) { | 946 | 949 | 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) { | 949 | 969 | cloned.publicExponent = new Uint8Array(cloned.publicExponent); | | 970 | + } | 950 | 971 | return cloned; | 951 | 972 | } | 952 | 973 |
| @@ -971,8 +992,8 @@ const { | 971 | 992 | return `CryptoKey ${inspect({ | 972 | 993 | type: getCryptoKeyType(this), | 973 | 994 | extractable: getCryptoKeyExtractable(this), | 974 | | - algorithm: getCryptoKeyAlgorithm(this), | 975 | | - usages: getCryptoKeyUsages(this), | | 995 | + algorithm: cloneAlgorithm(getCryptoKeyAlgorithm(this)), | | 996 | + usages: ArrayPrototypeSlice(getCryptoKeyUsages(this), 0), | 976 | 997 | }, opts)}`; | 977 | 998 | } | 978 | 999 |
| @@ -1008,6 +1029,12 @@ const { | 1008 | 1029 | class InternalCryptoKey extends NativeCryptoKey { | 1009 | 1030 | #slots; | 1010 | 1031 |
| | 1032 | + constructor(handle, algorithm, usagesMask, extractable) { | | 1033 | + if (algorithm !== undefined) | | 1034 | + algorithm = cloneInternalAlgorithm(algorithm); | | 1035 | + super(handle, algorithm, usagesMask, extractable); | | 1036 | + } | | 1037 | + | 1011 | 1038 | static { | 1012 | 1039 | getSlots = (key) => { | 1013 | 1040 | if (!key || typeof key !== 'object') | @@ -1017,6 +1044,7 @@ const { | 1017 | 1044 | if (cached !== undefined) return cached; | 1018 | 1045 | } | 1019 | 1046 | const slots = nativeGetCryptoKeySlots(key); | | 1047 | + slots[kSlotAlgorithm] = cloneInternalAlgorithm(slots[kSlotAlgorithm]); | 1020 | 1048 | key.#slots = slots; | 1021 | 1049 | return slots; | 1022 | 1050 | }; | |
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; | |
|