sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

ctf_sec - stakingContext.auraRewardPool.withdrawAndUnwrap boolean return value not handled in Boosted3TokenPoolUtils.sol and TwoTokenPoolUtils.sol #118

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

ctf_sec

medium

stakingContext.auraRewardPool.withdrawAndUnwrap boolean return value not handled in Boosted3TokenPoolUtils.sol and TwoTokenPoolUtils.sol

Summary

stakingContext.auraRewardPool.withdrawAndUnwrap boolean return value not handled in Boosted3TokenPoolUtils.sol and TwoTokenPoolUtils.sol

Vulnerability Detail

When calling function _unstakeAndExitPool,

the contract withdraw BPT tokens back to the vault for redemption

by calling

stakingContext.auraRewardPool.withdrawAndUnwrap(bptClaim, false);

however, the underlying call withdrawAndUnwrap returns boolean value, the contract does not handle the return value.

The see the interface of the IAuraRewardPool already indicate that the underlying call returns value

interface IAuraRewardPool {
    function withdrawAndUnwrap(uint256 amount, bool claim) external returns(bool);

and the underlying call with BaseRewardConvexPool.sol also returns the boolean

https://github.com/convex-eth/platform/blob/ece5998c54b0354a60f092e0dda1aa1f040ec8bd/contracts/contracts/BaseRewardPool.sol#L238

    function withdrawAndUnwrap(uint256 amount, bool claim) public updateReward(msg.sender) returns(bool){

Impact

Because there are stacks of external call:

Notional -> auraRewardPool -> BaseRewardPool,

without handling the return value explicitly, the transaction may risk fails silently.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/pool/Boosted3TokenPoolUtils.sol#L355

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/vaults/balancer/internal/pool/TwoTokenPoolUtils.sol#L310

Tool used

Manual Review

Recommendation

We recommend the project handle the return value when unstaking explicitly

bool unstaked = stakingContext.auraRewardPool.withdrawAndUnwrap(bptClaim, false);
require(unstaked, 'unstake failed');
jeffywu commented 1 year ago

@weitianjie2000

rayn731 commented 1 year ago

The fix looks good, it correctly handles the returned value from auraRewardPool.withdrawAndUnwrap