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

Solution for puppet-v2 added #6

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

Conversation

bart1e
Copy link

@bart1e bart1e commented Feb 2, 2023

This PR adds a solution for puppet-v2 challenge of Damn Vulnerable Defi.

The solution works this way:

  • it uses a custom deploy.js file for initialization instead of importing source files and instantiating classes
  • contracts deployed by deploy.js are used in PuppetV2PoolEchidna
  • PuppetV2PoolEchidna contains several helper functions used to interact with Uniswap, WETH, etc.
  • for this reason, PuppetV2PoolEchidna contract is treated as the attacker in the challenge

Short justification:

  • deploy.js is used, since otherwise, it would be necessary to import both Uniswap, DamnValuableToken, WETH and PuppetV2Pool, but they use several different versions of solidity. In fact, even Uniswap itself uses different versions for core and for periphery. So if I wanted to import all the files in a classic way, it would require migrating the entire Uniswap to a common solidity version (which is not only fixing imports in each file), which doesn't have too much sense in my opinion.
  • PuppetV2PoolEchidna is used as the attacker, because some functions require, for instance, approving some tokens that are not available at the contract deployment. puppet-v2.yaml uses the balanceContract option to supply the contract with the initial attacker's balance.

However, there are two issues:

  • even though echidna is provided just with 3 functions that it can call, if I allowed it to choose the amount in borrow, it wasn't capable of finding the solution. While manually setting the amount in getWeth function is not a big deal (it's logical to convert as much ETH to WETH as possible, since we cannot use plain ETH to borrow tokens), if we do the same in borow, it is almost like solving the challenge ourselves (that is, we cannot anymore treat echidna like a black box that will just find a solution if we didn't know it already).
  • since the test contract is also the attacker, it will not lose gas on transactions (in the original challenge, attacker has to spend some tiny amount of ETH to pay for the transactions). But, I do not think that it's a big deal - I just want to state it here for informational purposes.

Even, if you think that it doesn't make sense to create an exercise out of it, I still think the solution has its value, and might be even presented as an example in some new exercise in the building-secure-contracts repository as it shows how to handle more complex cases when we are unable to import source files for some reason.

But, if you think that it's suitable for an exercise, I will be happy to add relevant description and tips in building-secure-contracts. I may also add a solution for puppet challenge if you wish (similar approach will probably be needed, since we don't have Uniswap V1 sources).

I would be very grateful for any advice regarding the solution itself and the issues I have written about.

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