Skip to content

Commit e3ddb32

Browse files
panvaaduh95
authored andcommitted
crypto: harden WebCrypto against prototype pollution
Avoid re-wrapping native WebCrypto promises with PromiseResolve(), since resolving a promise can read its user-mutated constructor. Add a helper for chaining internal WebCrypto job promises without consulting Promise species state, and use it for intermediate job results. Also align JWK wrapping and unwrapping with the spec's fresh-global JSON handling by detaching internal JWK values from user prototypes. Use the internal UTF-8 encoder/decoder bindings instead of shared TextEncoder/TextDecoder prototype methods. Expand the WebCrypto prototype pollution regression test to cover SubtleCrypto methods, export formats, zero-length KDF results, JWK toJSON/kty pollution, and encoder/decoder prototype poisoning. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #63363 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
1 parent e04cd17 commit e3ddb32

13 files changed

Lines changed: 1010 additions & 155 deletions

‎lib/internal/crypto/diffiehellman.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ const {
55
FunctionPrototypeCall,
66
MathCeil,
77
ObjectDefineProperty,
8-
PromisePrototypeThen,
98
SafeSet,
109
TypedArrayPrototypeGetBuffer,
1110
Uint8Array,
@@ -66,6 +65,7 @@ const {
6665
const{
6766
getArrayBufferOrView,
6867
jobPromise,
68+
jobPromiseThen,
6969
toBuf,
7070
kHandle,
7171
}=require('internal/crypto/util');
@@ -409,7 +409,7 @@ function ecdhDeriveBits(algorithm, baseKey, length) {
409409
if(length===null)
410410
returnbits;
411411

412-
returnPromisePrototypeThen(bits,(bits)=>{
412+
returnjobPromiseThen(bits,(bits)=>{
413413
// If the length is not a multiple of 8 the nearest ceiled
414414
// multiple of 8 is sliced.
415415
constsliceLength=MathCeil(length/8);

‎lib/internal/crypto/util.js‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ const {
1414
ObjectEntries,
1515
ObjectKeys,
1616
ObjectPrototypeHasOwnProperty,
17+
PromisePrototypeThen,
1718
PromiseReject,
19+
PromiseWithResolvers,
1820
SafeMap,
1921
SafeSet,
2022
StringPrototypeToUpperCase,
@@ -77,6 +79,7 @@ const {
7779
emitExperimentalWarning,
7880
filterDuplicateStrings,
7981
lazyDOMException,
82+
setOwnProperty,
8083
}=require('internal/util');
8184

8285
const{
@@ -90,6 +93,7 @@ const {
9093
isDataView,
9194
isArrayBufferView,
9295
isAnyArrayBuffer,
96+
isPromise,
9397
}=require('internal/util/types');
9498

9599
constkHandle=Symbol('kHandle');
@@ -677,6 +681,69 @@ function jobPromise(getJob) {
677681
}
678682
}
679683

684+
// Temporarily shadow inherited then accessors on WebCrypto result objects.
685+
// Promise resolution reads "then" synchronously for thenable assimilation.
686+
// Returning an own undefined data property keeps that lookup from reaching
687+
// user-mutated prototypes.
688+
functionprepareWebCryptoResult(value){
689+
if((value===null||typeofvalue!=='object')&&
690+
typeofvalue!=='function'){
691+
returnfalse;
692+
}
693+
if(isPromise(value)||ObjectPrototypeHasOwnProperty(value,'then'))
694+
returnfalse;
695+
setOwnProperty(value,'then',undefined);
696+
returntrue;
697+
}
698+
699+
// Remove the temporary then property installed by prepareWebCryptoResult().
700+
functioncleanupWebCryptoResult(value){
701+
deletevalue.then;
702+
}
703+
704+
// Resolve a WebCrypto promise while inherited then accessors are shadowed.
705+
functionresolveWebCryptoResult(resolve,value){
706+
constshouldCleanupResult=prepareWebCryptoResult(value);
707+
try{
708+
resolve(value);
709+
}finally{
710+
if(shouldCleanupResult)
711+
cleanupWebCryptoResult(value);
712+
}
713+
}
714+
715+
// Run a WebCrypto promise reaction and settle the outer promise.
716+
functionsettleJobPromise(handler,resolve,reject,value,isRejected){
717+
try{
718+
if(typeofhandler==='function'){
719+
resolveWebCryptoResult(resolve,handler(value));
720+
}elseif(isRejected){
721+
reject(value);
722+
}else{
723+
resolveWebCryptoResult(resolve,value);
724+
}
725+
}catch(err){
726+
reject(err);
727+
}
728+
}
729+
730+
// Promise.prototype.then gets promise.constructor to determine the result
731+
// promise's species. These promises are internal WebCrypto intermediates, so
732+
// make that lookup stay on the promise itself instead of user-mutated state.
733+
functionjobPromiseThen(promise,onFulfilled,onRejected){
734+
const{
735+
promise: resultPromise,
736+
resolve,
737+
reject,
738+
}=PromiseWithResolvers();
739+
setOwnProperty(promise,'constructor',undefined);
740+
PromisePrototypeThen(
741+
promise,
742+
(value)=>settleJobPromise(onFulfilled,resolve,reject,value,false),
743+
(value)=>settleJobPromise(onRejected,resolve,reject,value,true));
744+
returnresultPromise;
745+
}
746+
680747
// In WebCrypto, the publicExponent option in RSA is represented as a
681748
// WebIDL "BigInteger"... that is, a Uint8Array that allows an arbitrary
682749
// number of leading zero bits. Our conventional APIs for reading
@@ -899,6 +966,9 @@ module.exports = {
899966
validateByteSource,
900967
validateKeyOps,
901968
jobPromise,
969+
jobPromiseThen,
970+
cleanupWebCryptoResult,
971+
prepareWebCryptoResult,
902972
validateMaxBufferLength,
903973
bigIntArrayToUnsignedBigInt,
904974
bigIntArrayToUnsignedInt,

0 commit comments

Comments
 (0)