Add input length checks for precompiled contracts #476
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ut in a few more places
…t require an evm instance
yerdua
changed the title
kevjue
reviewed
Sep 24, 2019
kevjue
approved these changes
Sep 24, 2019
…ons object, making the input larger than 96 bytes
…erant of inputs that are longer than the bytes that are actually read
kevjue
added a commit
that referenced
this pull request
Sep 25, 2019
* Check message address against signature (#477) * Check signing validator's address matches msg address * Add comments about use of sig data in tests * Try fix Circle build test failures * Try fix Circle build test failures, take 2 * tx price heap fix (#471) * contract_comm/currency/currency.go * fixed the txn price-sorted min-heap * merge master (#490) * Add precompiles to access validator set (#441) * set max gas to double of the charged gas for the 'intrinsic' smart contract calls (#472) * set max gas to double of the charged gas for the 'intrinsic' evm operations * addressed PR comments * addressed pr comment * Adds Prepared Certificates to ensure Istanbul liveness (#366) * Check message address against signature (#477) * Check signing validator's address matches msg address * Add comments about use of sig data in tests * Try fix Circle build test failures * Try fix Circle build test failures, take 2 * added new option --use-in-memory-discovery-table (#479) * added new option --use-in-memory-discovery-table * merge master (#489) * Adds Prepared Certificates to ensure Istanbul liveness (#366) * Check message address against signature (#477) * Check signing validator's address matches msg address * Add comments about use of sig data in tests * Try fix Circle build test failures * Try fix Circle build test failures, take 2 * Allow v4/v5 on a bootnode simultaneously, allow mobile to use discv5 (#454) * changes for isolating celo networks * changes to get unit tests working * changes to add salt in the discovery packets * removed checking for the ip address when handling a reply * added ping-ip-from-packet option to bootnode * for handling expected replies, don't filter on expected sender ip address if --pingIPFromPacket is used * Add v4 flag * Add unhandled and quicken docker builds * Add salt & logs * Add v4 flag * Add PeerDiscovery to mobile node config * Remove logs * Remove hardcoded bootnodes * Add salt & turn on discv5 * Delete hardcoded eth bootnodes * Make Discoveryv5 configurable * Lint * Add comment to bootnode v4/v5 handling * Change PeerDiscovery -> NoDiscovery * Remove mobile geth no discovery * Reduce istanbul default timeout, cap exp backoff (#475) * Reduce istanbul default timeout, cap exp backoff * Ensure round 0 timeout factors in block period * Sanitize logs (#495) * Change registry lookup and infrastructure lookup error logs to debug level * Sanitize logs regarding registry deployment * Change empty abi logging and comment * Lower log level from error to warning for potentially outdated istanbul messages * Change back to an error message * Add input length checks for precompiled contracts (#476) * add input length checks * check exact input length. add a new error for input length. check input in a few more places * add tests that verify the input-length checks for contracts that don't require an evm instance * fix formatting * add comments to explain input length checks * run the formatter * e2e transfer test was failing because it passes in a transaction options object, making the input larger than 96 bytes * e2e tests have revealed that our precompiled contracts need to be tolerant of inputs that are longer than the bytes that are actually read
kevjue
pushed a commit
that referenced
this pull request
Sep 26, 2019
* Log on ValidatorElections * merge master (#496) * Check message address against signature (#477) * Check signing validator's address matches msg address * Add comments about use of sig data in tests * Try fix Circle build test failures * Try fix Circle build test failures, take 2 * tx price heap fix (#471) * contract_comm/currency/currency.go * fixed the txn price-sorted min-heap * merge master (#490) * Add precompiles to access validator set (#441) * set max gas to double of the charged gas for the 'intrinsic' smart contract calls (#472) * set max gas to double of the charged gas for the 'intrinsic' evm operations * addressed PR comments * addressed pr comment * Adds Prepared Certificates to ensure Istanbul liveness (#366) * Check message address against signature (#477) * Check signing validator's address matches msg address * Add comments about use of sig data in tests * Try fix Circle build test failures * Try fix Circle build test failures, take 2 * added new option --use-in-memory-discovery-table (#479) * added new option --use-in-memory-discovery-table * merge master (#489) * Adds Prepared Certificates to ensure Istanbul liveness (#366) * Check message address against signature (#477) * Check signing validator's address matches msg address * Add comments about use of sig data in tests * Try fix Circle build test failures * Try fix Circle build test failures, take 2 * Allow v4/v5 on a bootnode simultaneously, allow mobile to use discv5 (#454) * changes for isolating celo networks * changes to get unit tests working * changes to add salt in the discovery packets * removed checking for the ip address when handling a reply * added ping-ip-from-packet option to bootnode * for handling expected replies, don't filter on expected sender ip address if --pingIPFromPacket is used * Add v4 flag * Add unhandled and quicken docker builds * Add salt & logs * Add v4 flag * Add PeerDiscovery to mobile node config * Remove logs * Remove hardcoded bootnodes * Add salt & turn on discv5 * Delete hardcoded eth bootnodes * Make Discoveryv5 configurable * Lint * Add comment to bootnode v4/v5 handling * Change PeerDiscovery -> NoDiscovery * Remove mobile geth no discovery * Reduce istanbul default timeout, cap exp backoff (#475) * Reduce istanbul default timeout, cap exp backoff * Ensure round 0 timeout factors in block period * Sanitize logs (#495) * Change registry lookup and infrastructure lookup error logs to debug level * Sanitize logs regarding registry deployment * Change empty abi logging and comment * Lower log level from error to warning for potentially outdated istanbul messages * Change back to an error message * Add input length checks for precompiled contracts (#476) * add input length checks * check exact input length. add a new error for input length. check input in a few more places * add tests that verify the input-length checks for contracts that don't require an evm instance * fix formatting * add comments to explain input length checks * run the formatter * e2e transfer test was failing because it passes in a transaction options object, making the input larger than 96 bytes * e2e tests have revealed that our precompiled contracts need to be tolerant of inputs that are longer than the bytes that are actually read
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This adds checks for the input length of some precompiled contracts, and throws a comprehensible error when the input is too short or too long. Without these checks it's possible to spam the contracts with empty input that crashes the node and doesn't take gas fees from the caller.
Tested
For the modified contracts that don't require an instance of
evm, unit tests for the input length are included in this PR. For the ones that do need an instance ofevm, liketransfer, the overhead in adding a unit test is high, so they'll be tested in end-to-end instead.Related issues