Skip to content

Commit

Permalink
🐛 Calldata validation (#17326)
Browse files Browse the repository at this point in the history
Co-authored-by: Dan J Miller <danjm.com@gmail.com>
Co-authored-by: Pedro Figueiredo <pedro.figueiredo@consensys.net>
Co-authored-by: brad-decker <bhdecker84@gmail.com>
  • Loading branch information
4 people authored Feb 1, 2023
1 parent b0ed6be commit fd81945
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 4 deletions.
28 changes: 28 additions & 0 deletions app/scripts/controllers/transactions/lib/util.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { ethErrors } from 'eth-rpc-errors';
import { Interface } from '@ethersproject/abi';
import abi from 'human-standard-token-abi';
import { addHexPrefix } from '../../../lib/util';
import {
TransactionEnvelopeType,
Expand Down Expand Up @@ -218,12 +220,38 @@ export function validateTxParams(txParams, eip1559Compatibility = true) {
);
}
break;
case 'data':
validateInputData(value);
ensureFieldIsString(txParams, 'data');
break;
default:
ensureFieldIsString(txParams, key);
}
});
}

/**
*
* @param {*} value
*/
export function validateInputData(value) {
if (value !== null) {
// Validate the input data
const hstInterface = new Interface(abi);
try {
hstInterface.parseTransaction({ data: value });
} catch (e) {
// Throw an invalidParams error if BUFFER_OVERRUN
/* eslint require-unicode-regexp: off */
if (e.message.match(/BUFFER_OVERRUN/)) {
throw ethErrors.rpc.invalidParams(
`Invalid transaction params: data out-of-bounds, BUFFER_OVERRUN.`,
);
}
}
}
}

/**
* Validates the {@code from} field in the given tx params
*
Expand Down
12 changes: 12 additions & 0 deletions app/scripts/controllers/transactions/lib/util.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ describe('txUtils', function () {
});
});

it('throws for data out of bounds buffer overrun', function () {
const sample = {
from: '0x1678a085c290ebd122dc42cba69373b5953b831d',
to: '0xfbb5595c18ca76bab52d66188e4ca50c7d95f77a',
data: '0xa9059cbb00000000000000000000000011b6A5fE2906F3354145613DB0d99CEB51f604C90000000000000000000000000000000000000000000000004563918244F400',
};
assert.throws(() => txUtils.validateTxParams(sample), {
message:
'Invalid transaction params: data out-of-bounds, BUFFER_OVERRUN.',
});
});

it('throws for missing "to" and "data"', function () {
const sample = {
from: '0x1678a085c290ebd122dc42cba69373b5953b831d',
Expand Down
8 changes: 4 additions & 4 deletions coverage-targets.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
// subset of files to check against these targets.
module.exports = {
global: {
branches: 20,
functions: 30,
lines: 57,
statements: 40,
branches: 22,
functions: 33.5,
lines: 62.25,
statements: 41.75,
},
transforms: {
branches: 100,
Expand Down

0 comments on commit fd81945

Please sign in to comment.