Repository navigation
ethereum: Fix adapter to filter proxy calls - #3219
Merged
Merged
Conversation
Without this the line that does `function_abi.decode_input(&call.input.0[4..])` was failing with the Err(InvalidData). For some reason some of the OpenSea Proxy contracts don't get recognized as DELEGATECALL, instead they are seen as CALLs, but you can identify them by the input having the wrong size since it's a fallback function call. A scenario to exemplify the problem, can be seen in this transaction: 0x4499dc75facb4cd27894196aac00bcb0ffbe3d64de87105878451d5637662f90 We would get the traces for the block 14043345 (0xD648D1) for the contract 0x7Be8076f4EA4A4AD08075C2508e481d6C946D12b. Then we would find two CALLs for the `atomicMatch_` function. However the first one would have the wrong size (fail at decoding) and the second one would be correct. That's because of the proxy and because of how fallbacks are compiled from Solidity (omitting the function signature, aka a feel bytes less).
evaporei
force-pushed
the
otavio/fix-ethabi
branch
from
February 4, 2022 04:09
58e938d to
c2729db
Compare
leoyvens
approved these changes
Feb 6, 2022
leoyvens
left a comment
Contributor
There was a problem hiding this comment.
Approved after a few refactors
Comment on lines
+309
to
+313
| if let None = v { | ||
| return false; | ||
| } | ||
|
|
||
| let signature = &v.unwrap().1; |
Contributor
There was a problem hiding this comment.
Suggested change
| if let None = v { | |
| return false; | |
| } | |
| let signature = &v.unwrap().1; | |
| let signature = match v { | |
| None => return false, | |
| Some(v) => &v.1, | |
| } |
Contributor
Author
There was a problem hiding this comment.
I've wrapped this suggestion with the self.contract_addresses_function_signatures.get(&call.to) function call, so there's no more v let definition.
| let correct_fn = signature.contains(&call.input.0[..4]); | ||
| // Not multiple of 32, wrong length to decode parameters. | ||
| // This is probably a delegatecall disguised as a regular call | ||
| // via fallback. |
Contributor
There was a problem hiding this comment.
Is there a reference you can link here if somebody wants to better understand how this happens? Maybe link an example trace in etherscan?
Contributor
Author
There was a problem hiding this comment.
I've improved the comment, can you check if it's good now please?
Comment on lines
+328
to
+330
| let wrong_input_size = (call.input.0.len() - 4) % 32 != 0; | ||
|
|
||
| correct_fn && !wrong_input_size |
Contributor
There was a problem hiding this comment.
Suggested change
| let wrong_input_size = (call.input.0.len() - 4) % 32 != 0; | |
| correct_fn && !wrong_input_size | |
| let correct_input_size = (call.input.0.len() - 4) % 32 == 0; | |
| correct_fn && correct_input_size |
evaporei
force-pushed
the
otavio/fix-ethabi
branch
from
February 6, 2022 18:53
b723457 to
b21ac40
Compare
evaporei
added a commit
that referenced
this pull request
Feb 8, 2022
* ethereum: Fix adapter to filter proxy calls Without this the line that does `function_abi.decode_input(&call.input.0[4..])` was failing with the Err(InvalidData). For some reason some of the OpenSea Proxy contracts don't get recognized as DELEGATECALL, instead they are seen as CALLs, but you can identify them by the input having the wrong size since it's a fallback function call. A scenario to exemplify the problem, can be seen in this transaction: 0x4499dc75facb4cd27894196aac00bcb0ffbe3d64de87105878451d5637662f90 We would get the traces for the block 14043345 (0xD648D1) for the contract 0x7Be8076f4EA4A4AD08075C2508e481d6C946D12b. Then we would find two CALLs for the `atomicMatch_` function. However the first one would have the wrong size (fail at decoding) and the second one would be correct. That's because of the proxy and because of how fallbacks are compiled from Solidity (omitting the function signature, aka a feel bytes less). * ethereum: Apply code review suggestions for the adapter * ethereum: Elaborate/improve comment about call input size
evaporei
added a commit
that referenced
this pull request
Feb 8, 2022
* ethereum: Fix adapter to filter proxy calls Without this the line that does `function_abi.decode_input(&call.input.0[4..])` was failing with the Err(InvalidData). For some reason some of the OpenSea Proxy contracts don't get recognized as DELEGATECALL, instead they are seen as CALLs, but you can identify them by the input having the wrong size since it's a fallback function call. A scenario to exemplify the problem, can be seen in this transaction: 0x4499dc75facb4cd27894196aac00bcb0ffbe3d64de87105878451d5637662f90 We would get the traces for the block 14043345 (0xD648D1) for the contract 0x7Be8076f4EA4A4AD08075C2508e481d6C946D12b. Then we would find two CALLs for the `atomicMatch_` function. However the first one would have the wrong size (fail at decoding) and the second one would be correct. That's because of the proxy and because of how fallbacks are compiled from Solidity (omitting the function signature, aka a feel bytes less). * ethereum: Apply code review suggestions for the adapter * ethereum: Elaborate/improve comment about call input size
This file contains hidden or 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
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.
Without this the line that does
function_abi.decode_input(&call.input.0[4..])was failing with theErr(InvalidData).For some reason some of the OpenSea Proxy contracts don't get recognized as DELEGATECALL, instead they are seen as CALLs, but you can identify them by the input having the wrong size since it's a fallback function call.
A scenario to exemplify the problem, can be seen in this transaction: 0x4499dc75facb4cd27894196aac00bcb0ffbe3d64de87105878451d5637662f90.
We would get the traces for the block
14043345(0xD648D1) for the contract0x7Be8076f4EA4A4AD08075C2508e481d6C946D12b. Then we would find two CALLs for theatomicMatch_function. However the first one would have the wrong size (fail at decoding) and the second one would be correct.That's because of the proxy and because of how fallbacks are compiled from Solidity (omitting the function signature, aka a feel bytes less).
Ethereum ABI (d)ecoding docs: https://docs.soliditylang.org/en/v0.8.11/abi-spec.html.
Fixes #3194