Fixed splitSignature computing wrong v for BytesLike (#700).

This commit is contained in:
Richard Moore 2020-01-11 03:34:00 -05:00
parent c84664953d
commit 4151c0eacd
No known key found for this signature in database
GPG Key ID: 665176BE8E9DC651
4 changed files with 103 additions and 85 deletions

View File

@ -335,18 +335,13 @@ export function splitSignature(signature: SignatureLike): Signature {
logger.throwArgumentError("invalid signature string; must be 65 bytes", "signature", signature);
}
// Get the r and s
// Get the r, s and v
result.r = hexlify(bytes.slice(0, 32));
result.s = hexlify(bytes.slice(32, 64));
// Reduce v to the canonical 27 or 28
result.v = bytes[64];
if (result.v !== 27 && result.v !== 28) {
result.v = 27 + (result.v % 2);
}
// Compute recoveryParam from v
result.recoveryParam = (result.v - 27);
result.recoveryParam = 1 - (result.v % 2);
// Compute _vs from recoveryParam and s
if (result.recoveryParam) { bytes[32] |= 0x80; }
@ -359,86 +354,76 @@ export function splitSignature(signature: SignatureLike): Signature {
result.recoveryParam = signature.recoveryParam;
result._vs = signature._vs;
// Normalize v into a canonical 27 or 28
if (result.v != null && !(result.v == 27 || result.v == 28)) {
result.v = 27 + (result.v % 2);
}
// Populate a missing v or recoveryParam if possible
if (result.recoveryParam == null && result.v != null) {
result.recoveryParam = 1 - (result.v % 2);
} else if (result.recoveryParam != null && result.v == null) {
result.v = 27 + result.recoveryParam;
} else if (result.recoveryParam != null && result.v != null) {
if (result.v !== 27 + result.recoveryParam) {
logger.throwArgumentError("signature v mismatch recoveryParam", "signature", signature);
}
}
// Make sure r and s are padded properly
if (result.r != null) { result.r = hexZeroPad(result.r, 32); }
if (result.s != null) { result.s = hexZeroPad(result.s, 32); }
// If the _vs is available, use it to populate missing s, v and recoveryParam
// and verify non-missing s, v and recoveryParam
if (result._vs != null) {
result._vs = hexZeroPad(result._vs, 32);
if (result._vs.length > 66) {
logger.throwArgumentError("signature _vs overflow", "signature", signature);
const vs = zeroPad(arrayify(result._vs), 32);
result._vs = hexlify(vs);
// Set or check the recid
const recoveryParam = ((vs[0] >= 128) ? 1: 0);
if (result.recoveryParam == null) {
result.recoveryParam = recoveryParam;
} else if (result.recoveryParam !== recoveryParam) {
logger.throwArgumentError("signature recoveryParam mismatch _vs", "signature", signature);
}
const vs = arrayify(result._vs);
const recoveryParam = ((vs[0] >= 128) ? 1: 0);
const v = 27 + result.recoveryParam;
// Use _vs to compute s
// Set or check the s
vs[0] &= 0x7f;
const s = hexlify(vs);
// Check _vs aggress with other parameters
if (result.s == null) {
result.s = s;
} else if (result.s !== s) {
logger.throwArgumentError("signature v mismatch _vs", "signature", signature);
}
}
// Use recid and v to populate each other
if (result.recoveryParam == null) {
if (result.v == null) {
result.v = v;
} else if (result.v !== v) {
logger.throwArgumentError("signature v mismatch _vs", "signature", signature);
logger.throwArgumentError("signature missing v and recoveryParam", "signature", signature);
} else {
result.recoveryParam = 1 - (result.v % 2);
}
if (recoveryParam == null) {
result.recoveryParam = recoveryParam;
} else if (result.recoveryParam !== recoveryParam) {
logger.throwArgumentError("signature recoveryParam mismatch _vs", "signature", signature);
} else {
if (result.v == null) {
result.v = 27 + result.recoveryParam;
} else if (result.recoveryParam !== (1 - (result.v % 2))) {
logger.throwArgumentError("signature recoveryParam mismatch v", "signature", signature);
}
}
// After all populating, both v and recoveryParam are still missing...
if (result.v == null && result.recoveryParam == null) {
logger.throwArgumentError("signature requires at least one of recoveryParam, v or _vs", "signature", signature);
if (result.r == null || !isHexString(result.r)) {
logger.throwArgumentError("signature missing or invalid r", "signature", signature);
} else {
result.r = hexZeroPad(result.r, 32);
}
// Check for canonical v
if (result.v !== 27 && result.v !== 28) {
logger.throwArgumentError("signature v not canonical", "signature", signature);
if (result.s == null || !isHexString(result.s)) {
logger.throwArgumentError("signature missing or invalid s", "signature", signature);
} else {
result.s = hexZeroPad(result.s, 32);
}
// Check that r and s are in range
if (result.r.length > 66 || result.s.length > 66) {
logger.throwArgumentError("signature overflow r or s", "signature", signature);
}
if (result._vs == null) {
const vs = arrayify(result.s);
if (vs[0] >= 128) {
logger.throwArgumentError("signature s out of range", "signature", signature);
}
if (result.recoveryParam) { vs[0] |= 0x80; }
result._vs = hexlify(vs);
const _vs = hexlify(vs);
if (result._vs) {
if (!isHexString(result._vs)) {
logger.throwArgumentError("signature invalid _vs", "signature", signature);
}
result._vs = hexZeroPad(result._vs, 32);
}
// Set or check the _vs
if (result._vs == null) {
result._vs = _vs;
} else if (result._vs !== _vs) {
logger.throwArgumentError("signature _vs mismatch v and s", "signature", signature);
}
}

View File

@ -72,6 +72,25 @@ export module TestCase {
finney_format?: string,
satoshi_format?: string
}
export type SignedTransaction = {
name: string;
accountAddress: string;
privateKey: string;
signedTransaction: string
unsignedTransaction: string;
signedTransactionChainId5: string
unsignedTransactionChainId5: string;
nonce: number;
gasLimit: string;
gasPrice: string;
to: string;
value: string;
data: string;
};
}
export function saveTests(tag: string, data: any) {

View File

@ -487,3 +487,37 @@ describe("Test nameprep", function() {
});
});
});
describe("Test Signature Manipulation", function() {
const tests: Array<TestCase.SignedTransaction> = loadTests("transactions");
tests.forEach((test) => {
it("autofills partial signatures - " + test.name, function() {
const address = ethers.utils.getAddress(test.accountAddress);
const hash = ethers.utils.keccak256(test.unsignedTransaction);
const data = ethers.utils.RLP.decode(test.signedTransaction);
const s = data.pop(), r = data.pop(), v = parseInt(data.pop().substring(2), 16);
const sig = ethers.utils.splitSignature({ r: r, s: s, v: v });
{
const addr = ethers.utils.recoverAddress(hash, {
r: r, s: s, v: v
});
assert.equal(addr, address, "Using r, s and v");
}
{
const addr = ethers.utils.recoverAddress(hash, {
r: sig.r, _vs: sig._vs
});
assert.equal(addr, address, "Using r, _vs");
}
{
const addr = ethers.utils.recoverAddress(hash, {
r: sig.r, s: sig.s, recoveryParam: sig.recoveryParam
});
assert.equal(addr, address, "Using r, s and recoveryParam");
}
});
});
});

View File

@ -61,30 +61,10 @@ describe('Test JSON Wallets', function() {
});
describe('Test Transaction Signing and Parsing', function() {
type TestCase = {
name: string;
accountAddress: string;
privateKey: string;
signedTransaction: string
unsignedTransaction: string;
signedTransactionChainId5: string
unsignedTransactionChainId5: string;
nonce: number;
gasLimit: string;
gasPrice: string;
to: string;
value: string;
data: string;
};
type TestCaseKey = 'nonce' | 'gasLimit' | 'gasPrice' | 'to' | 'value' | 'data';
function checkTransaction(parsedTransaction: any, test: TestCase): any {
function checkTransaction(parsedTransaction: any, test: TestCase.SignedTransaction): any {
let transaction: any = { };
['nonce', 'gasLimit', 'gasPrice', 'to', 'value', 'data'].forEach((key: TestCaseKey) => {
['nonce', 'gasLimit', 'gasPrice', 'to', 'value', 'data'].forEach((key: (keyof TestCase.SignedTransaction)) => {
let expected = test[key];
let value = parsedTransaction[key];
@ -124,7 +104,7 @@ describe('Test Transaction Signing and Parsing', function() {
}
let tests: Array<TestCase> = loadTests('transactions');
let tests: Array<TestCase.SignedTransaction> = loadTests('transactions');
tests.forEach((test) => {
it(('parses and signs transaction - ' + test.name), function() {
this.timeout(120000);