ethernaut icon indicating copy to clipboard operation
ethernaut copied to clipboard

Fix ether value pass through in Recovery factory

Open ckoopmann opened this issue 1 year ago • 1 comments

Currently the Factory contract of the Recovery level attempts to send a hardcoded 0.001 ether to the "lost" SimpleToken instance for recovery.

This has the following issues:

  1. If the user sets a msg.value of <0.001 ether and there are no other funds in the factory contract the level creation transaction will fail without revert reason.
  2. If the user sets a msg.value >0.001 ether the excess amount will be "stuck" in the factory contract and not be recovered upon solving the level
  3. If another user had previously committed the mistake outlined in 2. a third user can come along and obtain those left over funds by creating a level with msg.value < 0.001 ether

This PR fixes that by replacing the current logic in the Recovery factory with what is "standard" in the other level factories (such as the KingFactory) i.e.:

  1. Check if user has sent minimum eth amount upon level creation (0.001 ether) and revert with informative reason if not.
  2. Forward the whole msg.value amound to the level instance upon creation.

ckoopmann avatar Sep 05 '22 01:09 ckoopmann

If this PR is merged, it'd probably be best to do so before redeploying to a new testnet. See: https://github.com/OpenZeppelin/ethernaut/issues/340

ckoopmann avatar Sep 05 '22 01:09 ckoopmann

We are re-deploying to include an on-chain statistics system (we will also account for past data), so merging this now

xaler5 avatar Nov 17 '22 08:11 xaler5