sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

0x52 - UserData for balancer pool exits is malformed and will permanently trap users #129

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

0x52

high

UserData for balancer pool exits is malformed and will permanently trap users

Summary

UserData for balancer pool exits is malformed and will result in all withdrawal attempts failing, trapping the user permanently.

Vulnerability Detail

AuraSpell.sol#L184-L189

wAuraPools.getVault(lpToken).exitPool(
    IBalancerPool(lpToken).getPoolId(),
    address(this),
    address(this),
    IBalancerVault.ExitPoolRequest(tokens, minAmountsOut, "", false)
);

We see above that UserData is encoded as "". This is problematic as it doesn't contain the proper data for exiting the pool, causing all exit request to fail and trap the user permanently.

https://etherscan.io/address/0x5c6ee304399dbdb9c8ef030ab642b10820db8f56#code#F9#L50

function exactBptInForTokenOut(bytes memory self) internal pure returns (uint256 bptAmountIn, uint256 tokenIndex) {
    (, bptAmountIn, tokenIndex) = abi.decode(self, (WeightedPool.ExitKind, uint256, uint256));
}

UserData is decoded into the data shown above when using ExitKind = 0. Since the exit uses "" as the user data this will be decoded as 0 a.k.a EXACT_BPT_IN_FOR_ONE_TOKEN_OUT. This is problematic because the token index and bptAmountIn should also be encoded in user data for this kind of exit. Since it isn't the exit call will always revert and the user will be permanently trapped.

Impact

Users will be permanently trapped, unable to withdraw

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/main/blueberry-core/contracts/spell/AuraSpell.sol#L149-L224

Tool used

Manual Review

Recommendation

Encode the necessary exit data in userData

Gornutz commented 1 year ago

https://github.com/Blueberryfi/blueberry-core/commit/91e4fd639f484806552df202da3bd61c2b70c144

IAm0x52 commented 1 year ago

Fix contains excess logic that doesn't seem necessary. Waiting for additional clarification/changes