unstakeFor method in AmmGauge.sol does not respect CEI standard.

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/tokenomics/AmmGauge.sol#L131

Vulnerability details

Issue: unstakeFor method does not respect CEI which can lead to reentrancy attack.

Consequences: If a token that has a hook on transfer is used, an attacker can use unstakeFor to drain the AmmGauge of funds by calling unstakeFor via a custom contract that will reenter in the unstakeFor method once it receives the tokens on line 131.

File: AmmGauge.sol
118:     /**
119:      * @notice Unstake amount of AMM token and send to another account.
120:      * @param dst Account to which unstaked AMM tokens will be sent.
121:      * @param amount Amount of token to unstake.
122:      * @return `true` if success.
123:      */
124:     function unstakeFor(address dst, uint256 amount) public virtual override returns (bool) {
125:         require(amount > 0, Error.INVALID_AMOUNT);
126:         require(balances[msg.sender] >= amount, Error.INSUFFICIENT_BALANCE);
127: 
128:         _userCheckpoint(msg.sender);
129: 
130:         uint256 oldBal = IERC20(ammToken).balanceOf(address(this));
131:         IERC20(ammToken).safeTransfer(dst, amount);
132:         uint256 newBal = IERC20(ammToken).balanceOf(address(this));
133:         uint256 unstaked = oldBal - newBal;
134:         balances[msg.sender] -= unstaked;
135:         totalStaked -= unstaked;
136:         emit AmmUnstaked(msg.sender, ammToken, amount);
137:         return true;
138:     }

Proof of Concept

• AmmGauge balance = 100 tokens.
• Attacker has 10 tokens staked.
• He can call unstakeFor as src == dst and reenter by external call on line 381 until the balance is 0.
• The checks and updates of the balance after the external call, will pass once the balance is 0, making the attacker to successfully drain the contract.

Mitigations

Consider adding a reentrancy guard.

[chase-manning]

We do not support tokens that offer reentrancy opportunities

[GalloDaSballo]

See #158