sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

moneyversed - Reentrancy Attack vulnerability in SoftVault.sol #19

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

moneyversed

high

Reentrancy Attack vulnerability in SoftVault.sol

Summary

SoftVault.sol contract is vulnerable to reentrancy attacks on the withdraw() function, as it is calling a third-party contract's function before it has completed processing the user's request.

Vulnerability Detail

The function withdraw() in SoftVault.sol calls the external contract function doCutVaultWithdrawFee() from the IProtocolConfig interface. This external contract function's implementation can be delayed, and during this delay, the withdraw() function remains active, allowing for a reentrancy attack to occur.

Impact

An attacker can exploit this vulnerability to repeatedly execute the withdraw() function until the vault runs out of funds, which will cause the vault to fail. The attacker could also potentially manipulate the balance of the vault and/or the token balance of the contract, resulting in a loss of funds.

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/main/blueberry-core/contracts/vault/SoftVault.sol#L107

Tool used

Manual Review

Recommendation

It is recommended to avoid third-party function calls within a vulnerable function. For a safe approach, the external function should be called last to avoid the possibility of reentrancy attacks. A possible solution is to transfer the funds to the user first and then call the external function. Another solution is to use the Checks-Effects-Interactions pattern by making all external contract function calls after modifying the state. In this case, a possible solution is to change the function sequence and first transfer the funds to the user and then call the doCutVaultWithdrawFee() function to prevent any potential attack.

Another option is to use reentrancy guards that can be found in OpenZeppelin's ReentrancyGuardUpgradeable library. These guards prevent reentrancy attacks by using a modifier that blocks any recursive calls to the contract function while it is still executing.