Malt Finance contest - GiveMeTestEther's results

Handle

GiveMeTestEther

Vulnerability details

Impact

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Proof of Concept

2021-11-malt\src\contracts\Auction.sol: require(!auction.active, "Cannot claim tokens on an active auction"); 2021-11-malt\src\contracts\Auction.sol: require(redemption <= remaining.add(1), "Cannot claim more tokens than available"); 2021-11-malt\src\contracts\Auction.sol: require(auction.startingTime > 0, "No auction available for the given id");

2021-11-malt\src\contracts\AuctionEscapeHatch.sol: require(!active, "Cannot exit early on an active auction");

2021-11-malt\src\contracts\AuctionParticipant.sol: require(_index > replenishingIndex, "Cannot replenishingIndex to old value");

2021-11-malt\src\contracts\DAO.sol: require(block.timestamp >= getEpochStartTime(epoch + 1), "Cannot advance epoch until start of new epoch");

2021-11-malt\src\contracts\ERC20Permit.sol: require(balance >= value, "ERC20Permit: transfer amount exceeds balance"); 2021-11-malt\src\contracts\ERC20Permit.sol: require(balance >= value, "ERC20Permit: transfer amount exceeds balance");

2021-11-malt\src\contracts\Malt.sol: require(_service != address(0), "Cannot use address 0 as transfer service");

2021-11-malt\src\contracts\MovingAverage.sol: require(_sampleLength > 0, "Cannot have 0 second sample length");

2021-11-malt\src\contracts\libraries\SafeBurnMintableERC20.sol: require(abi.decode(returndata, (bool)), "SafeERC20: ERC20 operation did not succeed");

2021-11-malt\src\contracts\RewardSystem\RewardDistributor.sol: require(forfeited <= _globals.declaredBalance, "Cannot forfeit more than declared"); 2021-11-malt\src\contracts\RewardSystem\RewardDistributor.sol: require(amount <= _globals.declaredBalance, "Can't decrement more than total reward balance"); 2021-11-malt\src\contracts\RewardSystem\RewardDistributor.sol: require(_throttler != address(0), "Cannot set 0 address as throttler"); 2021-11-malt\src\contracts\RewardSystem\RewardDistributor.sol: require(_rewardMine != address(0), "Cannot set 0 address as rewardMine"); 2021-11-malt\src\contracts\RewardSystem\RewardDistributor.sol: require(_forfeitor != address(0), "Cannot set 0 address as forfeitor"); 2021-11-malt\src\contracts\RewardSystem\RewardDistributor.sol: require(_rewardToken != address(0), "Cannot set 0 address as reward Token"); 2021-11-malt\src\contracts\RewardSystem\RewardDistributor.sol: require(_updater != address(0), "Cannot set 0 address as focal length updater"); 2021-11-malt\src\contracts\RewardSystem\RewardOverflowPool.sol: require(_maxFulfillment <= 1000, "Can't have above 100% max fulfillment");

Tools Used

Python script

Recommended Mitigation Steps

• shorten revert strings

Handle

GiveMeTestEther

Vulnerability details

Impact

Cache the storage array length read (incl. subtraction of 1) to save the (warm) storage read and the subtraction each loop iteration.

https://eips.ethereum.org/EIPS/eip-2929 states WARM_STORAGE_READ_COST = 100 gas creating a local variable to store the storage array length will be way cheaper because it is stored on the stack (PUSH 3 gas (store) + POP 2 GAS (read) + other cheap ops)

Note: on https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/DexHandlers/UniswapHandler.sol#L304 there is already an access of the array length, so it would also save gas to already store the array length before the if statement in a local variable, because the assumption is that this function will almost only be called if the array is not empty. And after the if statement decrement the variable by 1.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/DexHandlers/UniswapHandler.sol#L315

Tools Used

Manual Analysis

Recommended Mitigation Steps

• replace to both occurrence of buyers.length - 1 as follows: -uint buyersLengthMinusOne = buyers.length - 1;

  for (uint i = 0; i < buyersLengthMinusOne; i = i + 1) { if (buyers[i] == _buyer) { // Replace the current item with the last and pop the last away. buyers[i] = buyers[buyersLengthMinusOne]; buyers.pop(); return; } }

Handle

GiveMeTestEther

Vulnerability details

Impact

Wasting gas by 0 initializing variables that are by default 0.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/Auction.sol 2021-11-malt\src\contracts\Auction.sol: uint256 secondsSinceStart = 0; 2021-11-malt\src\contracts\Auction.sol: uint256 premiumExcess = 0;

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/AuctionBurnReserveSkew.sol 2021-11-malt\src\contracts\AuctionBurnReserveSkew.sol: uint256 initialAuction = 0; 2021-11-malt\src\contracts\AuctionBurnReserveSkew.sol: uint256 participation = 0; 2021-11-malt\src\contracts\AuctionBurnReserveSkew.sol: uint256 initialIndex = 0; 2021-11-malt\src\contracts\AuctionBurnReserveSkew.sol: uint256 total = 0;

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/AuctionPool.sol 2021-11-malt\src\contracts\AuctionPool.sol: uint256 earnedReward = 0;

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/DAO.sol 2021-11-malt\src\contracts\DAO.sol: uint256 public epoch = 0;

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/ERC20VestedMine.sol 2021-11-malt\src\contracts\ERC20VestedMine.sol: uint256 earnedReward = 0;

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/FaucetTwo.sol 2021-11-malt\src\contracts\FaucetTwo.sol: uint256 balance = 0;

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/SwingTrader.sol 2021-11-malt\src\contracts\SwingTrader.sol: uint256 profit = 0;

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/RewardSystem/RewardDistributor.sol 2021-11-malt\src\contracts\RewardSystem\RewardDistributor.sol: uint256 vestedReward = 0;

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/RewardSystem/RewardThrottle.sol 2021-11-malt\src\contracts\RewardSystem\RewardThrottle.sol: uint256 totalAPR = 0; 2021-11-malt\src\contracts\RewardSystem\RewardThrottle.sol: uint256 totalProfit = 0; 2021-11-malt\src\contracts\RewardSystem\RewardThrottle.sol: uint256 totalBondedValue = 0;

Tools Used

Recommended Mitigation Steps

• don't 0 initialize

Handle

GiveMeTestEther

Vulnerability details

Impact

Storage variable focalID is read 2-3 times. First is a cold read (SLOAD = 800 gas) the others are warm reads (100 gas each, https://eips.ethereum.org/EIPS/eip-2929). Each cold read could be further reduced by storing the focalID in a local (stack) variable uint _focalID = focalID; (push + pop + others = not much gas)

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/RewardSystem/RewardDistributor.sol#L131

Tools Used

Recommended Mitigation Steps

• write uint _focalID = focalID; and replace the occurrence of focalID by this local var

Handle

GiveMeTestEther

Vulnerability details

Impact

Reuse the function argument in the event emit instead of the storage variable. This saves a SLOAD.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Timelock.sol#L66 https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Timelock.sol#L82

Tools Used

Recommended Mitigation Steps

• L76 write: emit NewDelay(_delay);
• L92: write: emit NewGracePeriod(_gracePeriod);

Handle

GiveMeTestEther

Vulnerability details

Impact

Move the initialization of sampleDiff below the if block to save gas in the case of return of the if block.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/MovingAverage.sol#L128

Tools Used

Handle

GiveMeTestEther

Vulnerability details

Impact

move the sampleDiff (L86) below the if statement L88 to save the declaration/initialization of sampleDiff in the case the if block gets executed and the function returns early

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/MovingAverage.sol#L86

Tools Used

Recommended Mitigation Steps

• move the declaration/initialization of of sampleDiff below the if statement

Handle

GiveMeTestEther

Vulnerability details

Impact

Reuse _sampleMemory instead of the storage variable sampleMemory in the condition statement of the loop to save gas.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/MovingAverage.sol#L60

Tools Used

Recommended Mitigation Steps

• rewrite L60 as: for (uint i = 0; i < _sampleMemory ; i++)

Handle

GiveMeTestEther

Vulnerability details

Impact

Remove L151 count = count + 1; and change L147 to uint256 index = _getIndexOfObservation(count++); so we save at least a SLOAD.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/AuctionBurnReserveSkew.sol#L143

Tools Used

Recommended Mitigation Steps

Remove L151 count = count + 1; and change L147 to uint256 index = _getIndexOfObservation(count++);

Handle

GiveMeTestEther

Vulnerability details

Impact

After teh if statement on L74, we have premiumExcess <= maxBurnSpend and therefore don't need to do a save subtraction (underflow check) on L80.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/AuctionBurnReserveSkew.sol#L69

Tools Used

Recommended Mitigation Steps

• rewrite L80 as: uint256 usableExcess = maxBurnSpend - premiumExcess;

Handle

GiveMeTestEther

Vulnerability details

Impact

Else block on L805 satisfies amountArbTokens <= unclaimedArbTokens and therefore no safe subtraction (underflow check) is needed (saves gas).

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L786

Tools Used

Recommended Mitigation Steps

• rewrite L805 as: unclaimedArbTokens = unclaimedArbTokens - amountArbTokens;

Handle

GiveMeTestEther

Vulnerability details

Vulnerability details

Impact

The else block on L241 satisfies amountTokens <= unclaimedArbTokens and therefore we don't need to do a safe subtraction (underflow check).

The else block on L247 satisfies amountTokens <= claimableArbitrageRewards and therefore we don't need to do a safe subtraction (underflow check).

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L216 https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L247

Tools Used

Recommended Mitigation Steps

• rewrite L241 as: unclaimedArbTokens = unclaimedArbTokens - amountTokens;
• rewrite L274 as: claimableArbitrageRewards = claimableArbitrageRewards - amountTokens-;

Handle

GiveMeTestEther

Vulnerability details

Impact

L209 nextCommitmentId = nextCommitmentId + 1; can be removed and L202 can be changed to nextCommitmentId++; to save a SLOAD

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L177

Tools Used

Manual Analysis

Recommended Mitigation Steps

Handle

GiveMeTestEther

Vulnerability details

Impact

Storage variable _activeEpoch is read a lot and can be cached in a local variable (epochTmp, maybe choose a better name =)) to save gas. Also the State struct can be loaded into a State storage (currentState, maybe also choose a better name) variable such that we don't have to access the storage array each time.

In the gas optimized code of "Recommended Mitigation Steps" section, the _activeEpoch only gets read once. Also note after the first "if" we write epoch to the storage variable "_activeEpoch" but then also write epoch to the local var "epochTmp" so we can use this local var in the whole function.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/RewardSystem/RewardThrottle.sol#L63

Tools Used

Recommended Mitigation Steps

function handleReward() public { uint256 balance = rewardToken.balanceOf(address(this));

uint256 epoch = dao.epoch();
uint256 epochTmp = _activeEpoch;
State storage currentState;

checkRewardUnderflow();

if (epoch > epochTmp) {
  _activeEpoch = epoch;
  epochTmp = epoch;

  currentState = _state[epochTmp];

  currentState.bondedValue = bonding.averageBondedValue(epochTmp); 
  currentState.profit = balance;
  currentState.rewarded = 0;
  currentState.throttle = throttle;
} else {
  currentState = _state[epochTmp];
  currentState.profit = currentState.profit.add(balance);
  currentState.throttle = throttle; 
}

// Fetch targetAPR before we update current epoch state
uint256 aprTarget = targetAPR();

// Distribute balance to the correct places
if (aprTarget > 0 && _epochAprGivenReward(epoch, balance) > aprTarget) {
  uint256 remainder = _getRewardOverflow(balance, aprTarget);
  emit RewardOverflow(epochTmp, remainder);

  if (remainder > 0) {
    rewardToken.safeTransfer(address(overflowPool), remainder);

    if (balance > remainder) {
      _sendToDistributor(balance - remainder, epochTmp);
    }
  }
} else {
  _sendToDistributor(balance, epochTmp);
}

emit HandleReward(epoch, balance);

}

Handle

GiveMeTestEther

Vulnerability details

Impact

Require statement conditions checks that no underflow can happen, therefore we don't need to use safe subtraction (underflow check).

• L275: require(amount <= _globals.declaredBalance, "Can't decrement more than total reward balance");
• L278: _globals.declaredBalance = _globals.declaredBalance.sub(amount); => Rewrite L278 as: _globals.declaredBalance = _globals.declaredBalance - amount;

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/RewardSystem/RewardDistributor.sol#L271

Tools Used

Recommended Mitigation Steps

• rewrite L278 as: _globals.declaredBalance = _globals.declaredBalance - amount;

Handle

GiveMeTestEther

Vulnerability details

Impact

L185: focalID = focalID + 1; can be removed and L197 can be adapted to: _resetFocalPoint(++focalID, newEndTime); to save at least one warm storage read (100 gas)

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/RewardSystem/RewardDistributor.sol#L180

Tools Used

Recommended Mitigation Steps

Handle

GiveMeTestEther

Vulnerability details

Impact

Require condition checks already underflow condition. There for no underflow check is needed.

• L154: require(forfeited <= _globals.declaredBalance, "Cannot forfeit more than declared");
• L156:_globals.declaredBalance = _globals.declaredBalance.sub(forfeited);

Therefore L156 can be written as: _globals.declaredBalance = _globals.declaredBalance - forfeited;

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/RewardSystem/RewardDistributor.sol#L153

Tools Used

Recommended Mitigation Steps

• L156 can be written as: _globals.declaredBalance = _globals.declaredBalance - forfeited;