Re-entrancy Doesn't Work after SafeMath Fix

Reentrancy is missing a SafeMath replacement on this line in Reentrance.withdraw(): balances[msg.sender] -= _amount;

I replaced it with balances[msg.sender] = balances[msg.sender].sub(_amount); but after doing so, I can’t get a working solution.

From what I can tell, the reentrace attack still occurs where Reentrance.withdraw() -> AttackingContract.fallback() -> Reentrance.withdraw()... and the balance of Reentrance gets drained to zero. However as the stack unwinds, when balances[msg.sender] gets to zero, it’ll cause the rest of the reentrance stack to revert as SafeMath will prevent the sender’s balance from underflowing. This means the attacking contract can’t actually withdraw more than it’s balance.

It works right now all because that line does not use SafeMath.

Is my analysis correct or is there still a way to exploit reentrance even with the SafeMath fix?

Hi, welcome! :wave:

I assume you are trying to solve this puzzle Re-entrancy | Ethernaut I think the key point of this puzzle is the re-entrancy rather than the safe math, it wants to make people known the pattern checks effects interactions, for more details, you can have a look at this documentation: https://fravoll.github.io/solidity-patterns/checks_effects_interactions.html

Of course, you should calculate a right amount when call (bool result, bytes memory data) = msg.sender.call.value(_amount)(""); this action should not revert, so all your re-entrancy actions can succeed.

1 Like

Yup! Totally understand the goal of the puzzle is reentrance and not safe math.

What actually happened was I had copied the code to run locally and it defaulted to solidity 0.8.x. I didn’t know at the time that 0.8 has over/under flow built in so while my reentrance code was actually correct, it wasn’t working due to my theory above. What was also frustrating was no errors were thrown because the call in withdraw would silently fail so I had no idea what the root cause was.

Now that I figured out the issue and that my reentrance code was actually correct, I’m determined to solving it with the safemath fix (if actually possible).

I don’t think using the correct amount would help. For example, Reentrace has a balance of 2 with 1 for AccountA and 1 for AttackingContract. Presumably you want to extract 2 using reentrace starting from AttackingContract. If that the balance deduction line is using safe math, it start reverting if balance[AttackingContract] goes below zero so you can’t actually get out more than 1.