Rubicon contest - ElKu's results

Lines of code

https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathHouse.sol#L327 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L265

Vulnerability details

In the setFeeTo function in BathToken, admin can change the address to which the withdrawal fees are sent. There is no timelock or multisig involved in the process. Which makes it easy for a sneaky admin to steal the funds behind everyone's back.

GAS OPTIMIZATION:

1. There are two places where we increment the state variable first and then return it as a function output. This is wasting gas. For example: last_stratTrade_id++; return last_stratTrade_id;

can be rewritten as:

   return ++last_stratTrade_id;

a. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathPair.sol#L206 b. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconMarket.sol#L436 2. There are lot of places where we could avoid reading multiple times from storage by just using a temporary variable in memory to hold the storage value. For example: (totalSupply == 0) ? shares = assets : shares = ( assets.mul(totalSupply) ).div(totalAssets());

Can be written as: uint256 ts = totalSupply; (ts == 0) ? shares = assets : shares = ( assets.mul(ts) ).div(totalAssets()); This particular example saves one extra storage read. Storage read is 100 gas while memory read is only 3 gas!

See the below reference links and the storage location we are reading multiple times. a. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathPair.sol#L177 for IBathHouse(bathHouse).reserveRatio() b. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L399 for totalSupply. c. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L457 for totalSupply. d. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L569 for totalSupply. e. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L703 for allowance[from][msg.sender]. f. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconMarket.sol#L829 for offers[offerId].buy_amt, offers[offerId].pay_amt g. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconMarket.sol#L868 for offers[offerId].buy_amt, offers[offerId].pay_amt h. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconMarket.sol#L909 for offers[offerId].buy_amt, offers[offerId].pay_amt i. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconMarket.sol#L933 for offers[offerId].buy_amt, offers[offerId].pay_amt j. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconMarket.sol#L1170 for _rank[id]. k. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/peripheral_contracts/BathBuddy.sol#L149 for "start".

3. Emit the function input instead of the storage variable. See references below: a. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconMarket.sol#L749 b. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconMarket.sol#L766

4. The if block can be outside the for loop. And the for loop can start with an "index" value of 1. This reduces the gas for checking "index == 0" in every iteration. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L86

5. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathPair.sol#L236 to Line 241 can be changed like this(see Line 226 for the equation):

   if (offer1.pay_amt != 0) { //changed here IBathToken(bathAssetAddress).cancel( info.askId, offer1.pay_amt //changed here ); } Same logic can be applied line 256 line 261. Modified if statement will look like this(see Line 227 for the equation):

   if (offer2.pay_amt != 0) { IBathToken(bathQuoteAddress).cancel( info.bidId, offer2.pay_amt ); }

https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathHouse.sol#L52 right under line 38 of the same file, to save one slot of storage.

c. https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathHouse.sol#L43 reserveRatio(as its max value is 100) can be declared as uint64 or even uint8, and combine with the declarations mentioned in the above point(b) to save one slot of storage.