Slingshot Finance contest - gpersoon's results

Handle

gpersoon

Vulnerability details

Impact

The function executeTrades() of Slingshot.sol calulates the initialBalance and finalBalance in a different way. If toToken == nativeToken then:

• initialBalance == executioner.balance;
• finalBalance == wrappedNativeToken.balanceOf(address(executioner))

This could lead to different problems:

1. If someone (accidentally) sends ETH to the executioner then: 1a) "finalBalance - initialBalance" could be negative, resulting in a revert 1b) finalBalance - initialBalance could be smaller than finalAmountMin, resulting in a revert (even if the expected amount of wrappedNativeToken has been received) So you could create a grieving attack by sending ETH to the executioner Then any future trades with toToken == nativeToken will most likely fail (until the nativeToken is removed with rescueTokens)
2. You can sweep any available wrappedNativeToken in the executioner because executioner.balance and thus initialBalance will usually be 0, so finalBalance is the entire amount of wrappedNativeToken in the executioner.

Proof of Concept

https://github.com/code-423n4/2021-10-slingshot/blob/f6e7a0a39e3267bbe3c7fe60d6074cbf54f5750f/contracts/Slingshot.sol#L81-L91

Tools Used

Recommended Mitigation Steps

Possible solutions:

1. Calculate initialBalance in the same way as finalBalance: if (toToken == nativeToken) { initialBalance = _getTokenBalance(address(wrappedNativeToken)); } else { initialBalance = _getTokenBalance(toToken); }

2. Sum the amount of nativeToken and wrappedNativeToken: function _getTokenBalance(address token) internal view returns (uint256 balance) { if (token == nativeToken) { balance = address(executioner).balance;
   token = address(wrappedNativeToken); } balance += IERC20(token).balanceOf(address(executioner)); }

3. Ignore the initialBalance because the executioner isn't supposed to have any tokens anyway