Some ERC20 do allow for user's control of execution. For example, ERC777 has tokensReceived() hook. This way, an ability to reenter can be executed with the usage of any such tokens.
StakerVault's unstakeFor do not control for reentrancy and uses balance difference to calculate how much to remove from user's accounted funds.
Suppose Bob has 10 tokens in the system, runs unstakeFor() and reenters with stakeFor() of the same 10 tokens on receiving them. 'unstaked' will become 0 as stakeFor() reimbursed account balance in full.
Bob obtains nothing, but his stakeFor() has increased the balances[account], which is now 20, while he spent no additional funds. This way he can rise the balance to cover all the funds Vault can provide and steal them with a final unstake [without reenrancy] for his full bloated balance.
Setting severity to high as that's the loss of funds of all other stakers.
Proof of Concept
StakerVault deals with arbitrary tokens from Backd pools:
chase-manning
commented
2 years ago
Lines of code
https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/StakerVault.sol#L364
Vulnerability details
Impact
Some ERC20 do allow for user's control of execution. For example, ERC777 has tokensReceived() hook. This way, an ability to reenter can be executed with the usage of any such tokens.
StakerVault's unstakeFor do not control for reentrancy and uses balance difference to calculate how much to remove from user's accounted funds.
Suppose Bob has 10 tokens in the system, runs unstakeFor() and reenters with stakeFor() of the same 10 tokens on receiving them. 'unstaked' will become 0 as stakeFor() reimbursed account balance in full.
Bob obtains nothing, but his stakeFor() has increased the balances[account], which is now 20, while he spent no additional funds. This way he can rise the balance to cover all the funds Vault can provide and steal them with a final unstake [without reenrancy] for his full bloated balance.
Setting severity to high as that's the loss of funds of all other stakers.
Proof of Concept
StakerVault deals with arbitrary tokens from Backd pools:
https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/StakerVault.sol#L30
User-facing unstakeFor do not have reentrancy control and accepts any balance difference as 'unstaked':
https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/StakerVault.sol#L364
https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/StakerVault.sol#L384
This way, whenever StakerVault's 'token' allows, an attacker can reenter on funds transfer:
https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/StakerVault.sol#L382
Recommended Mitigation Steps
Introduce reentrancy control, for example a well-tested nonReentrant modifier.