-
Notifications
You must be signed in to change notification settings - Fork 37
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: migrate GPv2Signing calldata manipulation tests to Foundry (#206)
## Description There is only one test involved, and the logic of it has changed in the upgrade process. The key difference is that we use Solidity version 0.8 in the tests, while the previous test used Solidity 0.7. The relevant part here is that contract calls now validate the padding in the calldata, making it impossible to successfully call `recoverOrderFromTrade` with manipulated calldata. Curiously, this should have been the case since Solidity ~0.5, however it wasn't properly enforced until Solidity 0.8 (see ethereum/solidity repo [here](https://github.com/ethereum/solidity/blob/5647d9910af304446073a380c49ff455dbe4c7ea/test/externalTests/gp2.sh#L98-L101) and discussion in the PR where this was introduced). ## Test Plan CI. ## Related Issues #120 --------- Co-authored-by: mfw78 <53399572+mfw78@users.noreply.github.com>
- Loading branch information
Showing
2 changed files
with
83 additions
and
71 deletions.
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| // SPDX-License-Identifier: LGPL-3.0-or-later | ||
| pragma solidity ^0.8; | ||
|
|
||
| import {Vm} from "forge-std/Test.sol"; | ||
|
|
||
| import {GPv2Order, GPv2Signing, IERC20} from "src/contracts/mixins/GPv2Signing.sol"; | ||
|
|
||
| import {GPv2SigningTestInterface, Helper} from "./Helper.sol"; | ||
|
|
||
| import {Bytes} from "test/libraries/Bytes.sol"; | ||
| import {Order} from "test/libraries/Order.sol"; | ||
| import {SettlementEncoder} from "test/libraries/encoders/SettlementEncoder.sol"; | ||
|
|
||
| contract CalldataManipulation is Helper { | ||
| using SettlementEncoder for SettlementEncoder.State; | ||
| using Bytes for bytes; | ||
|
|
||
| Vm.Wallet private trader; | ||
|
|
||
| constructor() { | ||
| trader = vm.createWallet("GPv2Signing.RecoverOrderFromTrade: trader"); | ||
| } | ||
|
|
||
| function test_invalid_EVM_transaction_encoding_does_not_change_order_hash( | ||
| Order.Fuzzed memory params, | ||
| uint256 paddingIndex | ||
| ) public { | ||
| // The variables for an EVM transaction are encoded in multiples of 32 | ||
| // bytes for all types except `string` and `bytes`. This extra padding | ||
| // is usually filled with zeroes by the library that creates the | ||
| // transaction. It can however be manually messed with. | ||
| // Since Solidity v0.8, using nonzero padding should cause the | ||
| // transaction to revert. | ||
| // Computing GPv2's orderUid requires copying 32-byte-encoded addresses | ||
| // from calldata to memory (buy and sell tokens), which are then hashed | ||
| // together with the rest of the order. This copying procedure would | ||
| // keep the padding bytes as they are in the (manipulated) calldata. | ||
| // If these 12 padding bits were not zero after copying, then the same | ||
| // order would end up with two different uids. This test shows that this | ||
| // is not the case by showing that such calldata manipulation causes the | ||
| // transaction to revert. | ||
|
|
||
| GPv2Order.Data memory order = Order.fuzz(params); | ||
|
|
||
| SettlementEncoder.State storage encoder = SettlementEncoder.makeSettlementEncoder(); | ||
| encoder.signEncodeTrade(vm, trader, order, domainSeparator, GPv2Signing.Scheme.Eip712, 0); | ||
|
|
||
| IERC20[] memory tokens = encoder.tokens(); | ||
| bytes memory encodedTransactionData = | ||
| abi.encodeCall(GPv2SigningTestInterface.recoverOrderFromTradeTest, (tokens, encoder.trades[0])); | ||
|
|
||
| // calldata encoding: | ||
| // - 4 bytes: signature | ||
| // - 32 bytes: pointer to first input value | ||
| // - 32 bytes: pointer to second input value | ||
| // - 32 bytes: first input value, array -> token array length | ||
| // - 32 bytes: first token address | ||
| uint256 startNumTokenWord = 4 + 2 * 32; | ||
| uint256 startFirstTokenWord = startNumTokenWord + 32; | ||
| uint256 encodedNumTokens = abi.decode(encodedTransactionData.slice(startNumTokenWord, 32), (uint256)); | ||
| require( | ||
| encodedNumTokens == ((order.sellToken == order.buyToken) ? 1 : 2), | ||
| "invalid test setup; has the transaction encoding changed?" | ||
| ); | ||
| bytes memory encodedFirstToken = encodedTransactionData.slice(startFirstTokenWord, 32); | ||
| uint256 tokenPaddingSize = 12; | ||
| for (uint256 i = 0; i < tokenPaddingSize; i++) { | ||
| require(encodedFirstToken[i] == bytes1(0), "invalid test setup; has the transaction encoding changed?"); | ||
| } | ||
| IERC20 token = IERC20(abi.decode(encodedFirstToken, (address))); | ||
| require(token == tokens[0], "invalid test setup; has the transaction encoding changed?"); | ||
| // Here we change a single padding byte, this is enough to make the | ||
| // transaction revert. | ||
| paddingIndex = paddingIndex % tokenPaddingSize; | ||
| encodedTransactionData[startFirstTokenWord + paddingIndex] = 0x42; | ||
| // Using low-level call because we want to call this function with data | ||
| // that is intentionally invalid. | ||
| // solhint-disable-next-line avoid-low-level-calls | ||
| (bool success,) = address(executor).call(encodedTransactionData); | ||
| assertFalse(success); | ||
| } | ||
| } |