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

[code423n4]

Lines of code

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

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 StakerVault of funds by calling unstakeFor via a custom contract that will reenter in the unstakeFor method once it receives the tokens on line 381.

 File: StakerVault.sol
381:         IERC20(token).safeTransfer(dst, amount);

Proof of Concept

• StakerVault 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 the transfer of the tokens after the balances are updated, furthermore, consider adding a reentrancy guard.

[chase-manning]

We do not support tokens that offer reentrancy opportunities

[GalloDaSballo]

Given the discussion for #19 I believe the finding to be valid but of non-critical severity