Re-entrancy Doesn't Work after SafeMath Fix

Security Ethernaut
1

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?

2

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