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

Reverse engineer attacker or vulnerable logic #40

Open
joaquinlpereyra opened this issue Dec 13, 2022 · 1 comment
Open

Reverse engineer attacker or vulnerable logic #40

joaquinlpereyra opened this issue Dec 13, 2022 · 1 comment

Comments

@joaquinlpereyra
Copy link
Collaborator

joaquinlpereyra commented Dec 13, 2022

Aim

There should be no need to use prank(attacker) in most scenarios, and there should be no need to hardcode payloads. We should be able to reproduce everything in the actual test.

Status

Bridges

  • Nomad Bridge: getPayload reproduces payload for any address
  • Roning Bridge: uses prank but OK, no interesting on-chain interactions, meat is offchain
  • Polynetwork: uses hardcoded bytecode from traces, no prank but attacker address needs to be hardcoded due to bytecode
  • Arbitrum Inbox: report, so no actual attacker address, attack is fully reproduced from scratch

Data Validation

  • Superfluid: implemented encode functions, nothing hardcoded
  • Bad Guys NFT: hardcoded attacker and merkle proof, needs logic to build merkle proof for any addr and set merkle root
  • Bond Olympus: OK, no hardcoding
  • Multichain Permit: OK, no hardcoding

Access Control

  • ⚠️ Sandbox: attacker/victim hardcoded, should work with any pair as long as victim has an NFT, could give it to them so test always works
  • ✅ ️ DAO Maker: OK, no hardcoding
  • 😞 Rikkeii: OK, but code could use some love so attack is more clear.
  • MBC Token: OK, address(this) is the attacker contract, could change it to anything
  • Temple DAO: OK, address(this) is the attacker contract, could change it to anything
  • Punk Protocol: OK, address(this) is the attacker contract, could change it to anything

Reentrancy

  • ✅ ️ Paraluni: OK, no hardcoding
  • ⚠️DFXFinance: Strong dependance on balance on an attacker address that is not in the test.
  • 😞 ️️ Fei Protocol: Needs love and work so assertGe asserts more things
  • ✅ ️️ Cream Finance: OK
  • ✅ ️️ Revest Finance: OK, uses attacker address but no prank, only to transfer loot
  • 😞 ️️ Hundred Finance: No hardcoding, but code is hard to understand. Missing asserts as token interactions are not clear.
  • 😞 Read only reeentrancy: totally theoretical so no hardcoding needed, but is missing asserts
@joaquinlpereyra
Copy link
Collaborator Author

This list is missing the Business Logic contracts, I can continue with it later. Meanwhile, I have done a PR which adds asserts to most of the contracts. Only those mentioned in the issue are missing (plus the business logic ones probably)

#41

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

No branches or pull requests

1 participant