Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: assert transferFrom call fails #60

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aa-eyup
Copy link

@aa-eyup aa-eyup commented Jun 3, 2023

Summary

Adds a test which checks the scenario where a malicious party signs an invocation message and sends it to a Delegatable ERC20 contract. The intention of the malicious party is to transfer funds from some other address to their own address.
Impacts test files only.

Motivation

  • Explicitly show some of the safe guards and security considerations of the Delegatable framework through testing
  • Demonstrate the importance of overwriting _msgSender() to check the last 20 bytes of calldata which would be set to the msg.sender (malicious actor's address) in the DelegatableCore._execute function
  • Check that just because there are no caveats to check that invoking transferFrom on an ERC20 contract still references the allowances of the msg.sender (or the signer of the first delegation created in the authority chain)

@aa-eyup aa-eyup force-pushed the test_ERC20_transferFrom branch from ed2b7a3 to c4a7fda Compare June 3, 2023 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant