Omniscia AmpleSense Audit

BalancerTrader Manual Review Findings

BalancerTrader Manual Review Findings

BTR-01M: Inexistent Approval of Vault

Description:

The vault of Balancer requires an approval to perform a trade if it is not utilizing internal balances as indicated by the false flags in the IVault.FundManagement struct. As a result, the operation will fail to execute at all times.

Example:

contracts/BalancerTrader.sol
58/**
59* @dev Caller must transfer the right amount of tokens to the trader (USDC will be replaced with ETH)
60 */
61function sellAMPLForEEFI(uint256 amount) external override returns (uint256 eefiAmount) {
62    amplToken.transferFrom(msg.sender, address(this), amount);
63    (uint256 usdcAmount,) = amplUsdc.swapExactAmountIn(address(amplToken), amount, address(usdcToken), 0, MAX_INT);
64    eefiAmount = vault.swap(IVault.SingleSwap(
65        eefiUsdcPoolID,
66        IVault.SwapKind.GIVEN_IN,
67        IAsset(address(usdcToken)),
68        IAsset(address(eefiToken)),
69        usdcAmount,
70        "0x"
71    ), IVault.FundManagement(
72        address(this),
73        false,
74        msg.sender,
75        false),
76        0, block.timestamp);
77    emit Sale_EEFI(amount, eefiAmount);
78}

Recommendation:

We strongly recommend approval to be set prior to each swap execution to ensure that it operates as expected and securely.

Alleviation:

The code was updated to properly approve the vault prior to each swap execution.

BTR-02M: Deprecated Native Asset Transfer

TypeSeverityLocation
Language SpecificMediumBalancerTrader.sol:L54

Description:

The transfer member accessible in payable variables forwards a fixed gas stipend for the operation which can change on upcoming EIPs as evidenced by f.e. EIP-2929.

Example:

contracts/BalancerTrader.sol
47/**
48* @dev Caller must transfer the right amount of tokens to the trader
49 */
50function sellAMPLForEth(uint256 amount) external override returns (uint256 ethAmount) {
51    amplToken.transferFrom(msg.sender, address(this), amount);
52    (ethAmount,) = amplEth.swapExactAmountIn(address(amplToken), amount, address(wethToken), 0, MAX_INT);
53    wethToken.withdraw(ethAmount);
54    msg.sender.transfer(ethAmount);
55    emit Sale_ETH(amount, ethAmount);
56}

Recommendation:

We strongly recommend a more robust payout method to be utilized such as the sendValue function of OpenZeppelin's Address library that forwards the currently available gas and is guaranteed to always be executable.

Alleviation:

The sendValue method of the OpenZeppelin Address library is now properly utilized.

BTR-03M: Inexistent Slippage Arguments

TypeSeverityLocation
Logical FaultMediumBalancerTrader.sol:L52, L63, L76

Description:

All trades performed by the contract do not enforce any slippage checks, rendering them completely prone to sandwhich attacks as well as potential flash-loan arbitrage opportunities.

Example:

contracts/BalancerTrader.sol
47/**
48* @dev Caller must transfer the right amount of tokens to the trader
49 */
50function sellAMPLForEth(uint256 amount) external override returns (uint256 ethAmount) {
51    amplToken.transferFrom(msg.sender, address(this), amount);
52    (ethAmount,) = amplEth.swapExactAmountIn(address(amplToken), amount, address(wethToken), 0, MAX_INT);
53    wethToken.withdraw(ethAmount);
54    msg.sender.transfer(ethAmount);
55    emit Sale_ETH(amount, ethAmount);
56}
57

58/**
59* @dev Caller must transfer the right amount of tokens to the trader (USDC will be replaced with ETH)
60 */
61function sellAMPLForEEFI(uint256 amount) external override returns (uint256 eefiAmount) {
62    amplToken.transferFrom(msg.sender, address(this), amount);
63    (uint256 usdcAmount,) = amplUsdc.swapExactAmountIn(address(amplToken), amount, address(usdcToken), 0, MAX_INT);
64    eefiAmount = vault.swap(IVault.SingleSwap(
65        eefiUsdcPoolID,
66        IVault.SwapKind.GIVEN_IN,
67        IAsset(address(usdcToken)),
68        IAsset(address(eefiToken)),
69        usdcAmount,
70        "0x"
71    ), IVault.FundManagement(
72        address(this),
73        false,
74        msg.sender,
75        false),
76        0, block.timestamp);
77    emit Sale_EEFI(amount, eefiAmount);
78}

Recommendation:

We advise this trait to be carefully assessed and potentially mitigated by introducing new arguments to the functions as in their current state they would be exploitable by arbitreurs.

Alleviation:

Both linked functions had a new argument introuduced that is properly utilized to ensure expected behaviour and a minimum return of the swaps performed.

BTR-04M: Potential for Lock of Ether

Description:

The receive fallback function does not validate its caller, meaning anyone can directly send ether to the contract and have it permanently locked.

Example:

contracts/BalancerTrader.sol
43receive() external payable {
44

45}
46

47/**
48* @dev Caller must transfer the right amount of tokens to the trader
49 */
50function sellAMPLForEth(uint256 amount) external override returns (uint256 ethAmount) {
51    amplToken.transferFrom(msg.sender, address(this), amount);
52    (ethAmount,) = amplEth.swapExactAmountIn(address(amplToken), amount, address(wethToken), 0, MAX_INT);
53    wethToken.withdraw(ethAmount);
54    msg.sender.transfer(ethAmount);
55    emit Sale_ETH(amount, ethAmount);
56}

Recommendation:

We strongly recommend a require check to be introduced here that ensures msg.sender is equivalent to the wethToken to ensure that only unwrapped WETH is ever sent to the contract in the sellAMPLForEth call path.

Alleviation:

A require check was introduced ensuring that only the wethToken sends native assets to the contract alleviating this exhibit.

Prev

AmplesenseVault.sol (AVT-M)

Next

Distribute.sol (DIS-M)

On this page