Slingshot Finance contest - 0x0x0x's results

Handle

0x0x0x

Vulnerability details

Impact

!= 0 is a cheaper operation compared to > 0, when dealing with uint.

Occurences

contracts/Slingshot.sol:74:        require(finalAmountMin > 0, "Slingshot: finalAmountMin cannot be zero");
contracts/Slingshot.sol:75:        require(trades.length > 0, "Slingshot: trades cannot be empty");
contracts/module/IUniswapModule.sol:28:        require(path.length > 0, "UniswapModule: path length must be >0");

Tools Used

grep

Handle

0x0x0x

Vulnerability details

Impact

Gas optimization

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

At ModuleRegistry.setSlingshot(), old slingshot adress is saved to variable oldAdress. oldAdress is only used to emit the change of slingshot adress. It is not required to save the oldAdress to a variable. L71-73:

        address oldAddress = slingshot;
        slingshot = _slingshot;
        emit NewSlingshot(oldAddress, _slingshot);

Recommended Mitigation Steps

Replace with:

        emit NewSlingshot(slingshot, _slingshot);
        slingshot = _slingshot;

Tools Used

Manual analysis

Handle

0x0x0x

Vulnerability details

Impact

Length is read multiple times. The length can be saved to a variable to avoid reading it repeatedly and waste gas.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Occurences

''' contracts/Executioner.sol: for(uint256 i = 0; i < trades.length; i++) { contracts/ModuleRegistry.sol: for (uint256 i = 0; i < _moduleAddresses.length; i++) { contracts/ModuleRegistry.sol: for (uint256 i = 0; i < _moduleAddresses.length; i++) { contracts/Slingshot.sol: for(uint256 i = 0; i < trades.length; i++) { '''

Tools Used

Grep

Recommended Mitigation Steps

Cache .length to and int before going into loop.