sherlock-audit / 2023-07-blueberry-judging

2 stars 1 forks source link

feelereth - Reentrancy vulnerability in the withdraw() function #95

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

feelereth

high

Reentrancy vulnerability in the withdraw() function

Summary

The contract inherits from ReentrancyGuard, which helps prevent reentrancy attacks on the main functions. However, _ensureApprove() is called after redeeming tokens from bToken, which could be vulnerable to a reentrancy attack. An attacker can initiate a recursive call back into withdraw() before the tokens are transferred out, draining more tokens

Vulnerability Detail

The nonReentrant modifier prevents reentrancy on withdraw() itself, but _ensureApprove() could be called recursively before the tokens are transferred out.

An attacker could: Call withdraw() with a large shareAmount bToken.redeem() succeeds and transfers tokens to the vault Before the tokens are sent out, _ensureApprove() is called The attacker calls withdraw() again recursively More tokens are redeemed from bToken before the previous tokens are sent out Repeat to drain tokens

Impact

An attacker can drain a lot more tokens than intended from a single call to withdraw().

Code Snippet

https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/vault/SoftVault.sol#L128-L129 https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/vault/SoftVault.sol#L133-L137

Tool used

Manual Review

Recommendation

_ensureApprove() should be called before redeeming from bToken

sherlock-admin2 commented 1 year ago

3 comment(s) were left on this issue during the judging contest.

0xyPhilic commented:

invalid because _ensureApproval is an internal function and can't be re-entered

darkart commented:

it esnuring approval they have similar design to OZ SafeERC20

Kral01 commented:

this is a good pattern recommendation but need to provide a valid PoC