Skip to content

Commit

Permalink
fix(secp256k1): prevent vulnerabilities in consumers which don't vali…
Browse files Browse the repository at this point in the history 
…date input lengths
  • Loading branch information
@bitjson
bitjson committed Jan 31, 2022
1 parent 411ff61 commit 7fc75c9
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 8 deletions.
81 changes: 80 additions & 1 deletion src/lib/crypto/secp256k1.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,10 @@ test('[crypto] secp256k1.signMessageHashCompact', async (t) => {
secp256k1.signMessageHashCompact(privkey, messageHash),
sigCompact
);
t.notDeepEqual(
secp256k1.signMessageHashCompact(privkey, Uint8Array.of()),
sigCompact
);
t.throws(() =>
secp256k1.signMessageHashCompact(secp256k1OrderN, messageHash)
);
Expand Down Expand Up @@ -765,6 +769,10 @@ test('[fast-check] [crypto] secp256k1.signMessageHashCompact', async (t) => {
test('[crypto] secp256k1.signMessageHashDER', async (t) => {
const secp256k1 = await secp256k1Promise;
t.deepEqual(secp256k1.signMessageHashDER(privkey, messageHash), sigDER);
t.notDeepEqual(
secp256k1.signMessageHashDER(privkey, Uint8Array.of()),
sigDER
);
t.throws(() => secp256k1.signMessageHashDER(secp256k1OrderN, messageHash));
});

Expand Down Expand Up @@ -967,6 +975,27 @@ test('[crypto] secp256k1.verifySignatureCompact', async (t) => {
messageHash
)
);
t.false(
secp256k1.verifySignatureCompact(
sigCompactHighS,
pubkeyCompressed,
Uint8Array.of()
)
);
t.true(
secp256k1.verifySignatureCompact(
sigCompactHighS,
pubkeyCompressed,
messageHash
)
);
t.false(
secp256k1.verifySignatureCompact(
Uint8Array.of(),
pubkeyCompressed,
messageHash
)
);
});

test('[fast-check] [crypto] secp256k1.verifySignatureCompact', async (t) => {
Expand Down Expand Up @@ -1023,6 +1052,27 @@ test('[crypto] secp256k1.verifySignatureCompactLowS', async (t) => {
messageHash
)
);
t.false(
secp256k1.verifySignatureCompactLowS(
Uint8Array.of(),
pubkeyCompressed,
messageHash
)
);
t.true(
secp256k1.verifySignatureCompactLowS(
sigCompact,
pubkeyCompressed,
messageHash
)
);
t.false(
secp256k1.verifySignatureCompactLowS(
sigCompact,
pubkeyCompressed,
Uint8Array.of()
)
);
});

test('[fast-check] [crypto] secp256k1.verifySignatureCompactLowS', async (t) => {
Expand Down Expand Up @@ -1076,13 +1126,35 @@ test('[crypto] secp256k1.verifySignatureDER', async (t) => {
t.true(
secp256k1.verifySignatureDER(sigDERHighS, pubkeyCompressed, messageHash)
);
t.false(
secp256k1.verifySignatureDER(Uint8Array.of(), pubkeyCompressed, messageHash)
);
t.true(
secp256k1.verifySignatureDER(sigDERHighS, pubkeyCompressed, messageHash)
);
t.false(
secp256k1.verifySignatureDER(sigDERHighS, pubkeyCompressed, Uint8Array.of())
);
});

test('[crypto] secp256k1.verifySignatureDERLowS', async (t) => {
const secp256k1 = await secp256k1Promise;
t.true(
secp256k1.verifySignatureDERLowS(sigDER, pubkeyCompressed, messageHash)
);
t.false(
secp256k1.verifySignatureDERLowS(
Uint8Array.of(),
pubkeyCompressed,
messageHash
)
);
t.true(
secp256k1.verifySignatureDERLowS(sigDER, pubkeyCompressed, messageHash)
);
t.false(
secp256k1.verifySignatureDERLowS(sigDER, pubkeyCompressed, Uint8Array.of())
);
const pubkeyWithBrokenEncoding = pubkeyCompressed.slice().fill(0, 0, 1);
t.false(
secp256k1.verifySignatureDERLowS(
Expand Down Expand Up @@ -1193,7 +1265,7 @@ test('[fast-check] [crypto] secp256k1.signMessageHashSchnorr', async (t) => {
});
});

test('[crypto] secp256k1.verifySchnorr', async (t) => {
test('[crypto] secp256k1.verifySignatureSchnorr', async (t) => {
const secp256k1 = await secp256k1Promise;
t.true(
secp256k1.verifySignatureSchnorr(
Expand All @@ -1202,6 +1274,13 @@ test('[crypto] secp256k1.verifySchnorr', async (t) => {
schnorrMsgHash
)
);
t.false(
secp256k1.verifySignatureSchnorr(
Uint8Array.of(),
pubkeyCompressed,
schnorrMsgHash
)
);
const pubkeyWithBrokenEncoding = pubkeyCompressed.slice().fill(0, 0, 1);
t.false(
secp256k1.verifySignatureSchnorr(
Expand Down
27 changes: 20 additions & 7 deletions src/lib/crypto/secp256k1.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,15 @@ const wrapSecp256k1Wasm = (
// eslint-disable-next-line no-bitwise, @typescript-eslint/no-magic-numbers
const lengthPtrView32 = lengthPtr >> 2;

const cloneAndPad = (value: Uint8Array, expectedLength: number) => {
const zeroPaddedValue = new Uint8Array(expectedLength);
zeroPaddedValue.set(value);
return zeroPaddedValue;
};

const parsePublicKey = (publicKey: Uint8Array) => {
secp256k1Wasm.heapU8.set(publicKey, publicKeyScratch);
const paddedPublicKey = cloneAndPad(publicKey, ByteLength.maxPublicKey);
secp256k1Wasm.heapU8.set(paddedPublicKey, publicKeyScratch);
return (
secp256k1Wasm.pubkeyParse(
contextPtr,
Expand Down Expand Up @@ -128,7 +135,8 @@ const wrapSecp256k1Wasm = (
};

const parseSignature = (signature: Uint8Array, isDer: boolean) => {
secp256k1Wasm.heapU8.set(signature, sigScratch);
const paddedSignature = cloneAndPad(signature, ByteLength.maxECDSASig);
secp256k1Wasm.heapU8.set(paddedSignature, sigScratch);
return isDer
? secp256k1Wasm.signatureParseDER(
contextPtr,
Expand Down Expand Up @@ -177,7 +185,8 @@ const wrapSecp256k1Wasm = (
};

const fillPrivateKeyPtr = (privateKey: Uint8Array) => {
secp256k1Wasm.heapU8.set(privateKey, privateKeyPtr);
const paddedPrivateKey = cloneAndPad(privateKey, ByteLength.privateKey);
secp256k1Wasm.heapU8.set(paddedPrivateKey, privateKeyPtr);
};

const zeroOutPtr = (pointer: number, bytes: number) => {
Expand Down Expand Up @@ -219,7 +228,8 @@ const wrapSecp256k1Wasm = (
};

const fillMessageHashScratch = (messageHash: Uint8Array) => {
secp256k1Wasm.heapU8.set(messageHash, messageHashScratch);
const paddedMessageHash = cloneAndPad(messageHash, ByteLength.messageHash);
secp256k1Wasm.heapU8.set(paddedMessageHash, messageHashScratch);
};

const normalizeSignature = () => {
Expand Down Expand Up @@ -352,7 +362,8 @@ const wrapSecp256k1Wasm = (
signature: Uint8Array
) => {
fillMessageHashScratch(messageHash);
secp256k1Wasm.heapU8.set(signature, schnorrSigPtr);
const paddedSignature = cloneAndPad(signature, ByteLength.schnorrSig);
secp256k1Wasm.heapU8.set(paddedSignature, schnorrSigPtr);
return (
secp256k1Wasm.schnorrVerify(
contextPtr,
Expand Down Expand Up @@ -412,7 +423,8 @@ const wrapSecp256k1Wasm = (
messageHash: Uint8Array
) => {
fillMessageHashScratch(messageHash);
secp256k1Wasm.heapU8.set(signature, sigScratch);
const paddedSignature = cloneAndPad(signature, ByteLength.maxECDSASig);
secp256k1Wasm.heapU8.set(paddedSignature, sigScratch);
if (
secp256k1Wasm.recoverableSignatureParse(
contextPtr,
Expand Down Expand Up @@ -542,7 +554,8 @@ const wrapSecp256k1Wasm = (
*/
if (randomSeed !== undefined) {
const randomSeedPtr = messageHashScratch;
secp256k1Wasm.heapU8.set(randomSeed, randomSeedPtr);
const paddedRandomSeed = cloneAndPad(randomSeed, ByteLength.randomSeed);
secp256k1Wasm.heapU8.set(paddedRandomSeed, randomSeedPtr);
secp256k1Wasm.contextRandomize(contextPtr, randomSeedPtr);
zeroOutPtr(randomSeedPtr, ByteLength.randomSeed);
}
Expand Down

0 comments on commit 7fc75c9

Please sign in to comment.