Sherlock contest - p4st13r4's results

Handle

p4st13r4

Vulnerability details

Impact

In sweepTokens() of SherBuy.sol , the variable i is incremented using i++. It is known that using ++i costs less gas per iteration than i++.

Proof of Concept

• https://github.com/code-423n4/2022-01-sherlock/blob/main/contracts/SherBuy.sol#L186
• https://github.com/code-423n4/2022-01-sherlock/blob/main/contracts/managers/Manager.sol#L46
• https://github.com/code-423n4/2022-01-sherlock/blob/main/contracts/managers/Manager.sol#L46
• https://github.com/code-423n4/2022-01-sherlock/blob/main/contracts/managers/SherlockClaimManager.sol#L234
• https://github.com/code-423n4/2022-01-sherlock/blob/main/contracts/managers/SherlockClaimManager.sol#L505
• https://github.com/code-423n4/2022-01-sherlock/blob/main/contracts/managers/SherlockClaimManager.sol#L505

Tools Used

Editor

Recommended Mitigation Steps

Replace i++ with ++i

Handle

p4st13r4

Vulnerability details

Impact

Starting from updateSherDistributionManager() and up to updateYieldStrategy(), all the functions are implemented so that the affected state variables is read twice from the storage. In order to consume less gas avoiding one of the two storage reads, the variable could be cached

Proof of Concept

The affected functions could be implemented like that:

function updateSherDistributionManager(ISherDistributionManager newSherDistributionManager)
    external
    override
    onlyOwner
  {
    ISherDistributionManager memory oldSherDistributionManager = sherDistributionManager;
    if (address(newSherDistributionManager) == address(0)) revert ZeroArgument();
    if (oldSherDistributionManager == newSherDistributionManager) revert InvalidArgument();

    emit SherDistributionManagerUpdated(oldSherDistributionManager, newSherDistributionManager);
    sherDistributionManager = newSherDistributionManager;
  }

Tools Used

Editor

Recommended Mitigation Steps

As explained in the example above