sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

cuthalion0x - `AuraSpell`'s Balancer pool exit will always revert #147

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

cuthalion0x

high

AuraSpell's Balancer pool exit will always revert

Summary

In AuraSpell.closePositionFarm, the call to BalancerVault.exitPool is not properly encoded and will always revert.

Vulnerability Detail

All Balancer Vault operations include an input argument called userData. This is optional for all known swaps, but it is very much required for all known joins and exits. In AuraSpell.closePositionFarm, a call to BalancerVault.exitPool leaves the userData empty. As a result, the exit will always revert, and it will be impossible to close a position.

Impact

It is not possible to close a position using the AuraSpell.

Code Snippet

spell/AuraSpell.sol#L184-189, annotated below for clarity.

wAuraPools.getVault(lpToken).exitPool(
    IBalancerPool(lpToken).getPoolId(),
    address(this),
    address(this),
    IBalancerVault.ExitPoolRequest(tokens, minAmountsOut, "", false) // @audit-info The "" here is empty `userData`.
);

Tool used

Manual Review

Recommendation

First, create an integration test for the Aura spell. There are tests for Convex, Curve, and Ichi, but not Aura. A test would likely have caught this issue.

To fix the issue, consult the Balancer docs on pool exits. The safest exit type is the "proportional" exit, whose userData looks like:

[uint256, uint256]
[EXACT_BPT_IN_FOR_TOKENS_OUT, bptAmountIn]

Note that if we choose the proportional exit, we will get all underlying tokens out and will have to deal with each one.

Duplicate of #129