Behodler contest - Jujic's results

Handle

Jujic

Vulnerability details

1. Long Revert Strings

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. ##Proof of Concept

require(morgothApprover.approved(token), "MORGOTH: token not approved for listing on Behodler")

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/Proposals/UpdateMultipleSoulConfigProposal.sol#L49

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/FlashGovernanceArbiter.sol#L71

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/FlashGovernanceArbiter.sol#L79

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/FlashGovernanceArbiter.sol#L147

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L22

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L24

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L240

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L270

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L277

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L368

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/ProposalFactory.sol#L34 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Flan.sol#L96-L97

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Flan.sol#L105

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/FlanBackstop.sol#L102

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/UniswapHelper.sol#L94

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/UniswapHelper.sol#L133

Tools

https://planetcalc.com/9029/

Recommended Mitigation Steps

Shorten the revert strings to fit in 32 bytes.

2. > 0 can be replaced with != 0 for gas optimisation

Impact

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

Proof of Concept

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/FlashGovernanceArbiter.sol#L145 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/FlashGovernanceArbiter.sol#L168-L169 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L240 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L280 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L287 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L357 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L393 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Limbo.sol#L326 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Limbo.sol#L387-L388 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Limbo.sol#L457 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Limbo.sol#L460 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Limbo.sol#L514 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Limbo.sol#L532

require(actualEyeBalance > 0, "LimboDAO: No EYE");

Tools

Remix

Recommended Mitigation Steps

Replace > 0 with !=

3. Loops can be more efficient

Impact

1. A more efficient for loop index proceeding where you could use unchecked{++i}
2. The local variable used as for loop index need not be initialized to 0 because the default value is 0. Avoiding this anti-pattern can save a few opcodes and therefore a tiny bit of gas.

Proof of Concept

for (uint256 i = 0; i < sushiLPs.length; i++) 

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L212

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L217

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/Proposals/UpdateMultipleSoulConfigProposal.sol#L64

Tools

Remix

Recommended Mitigation Steps

1. Here you could use unchecked{++i} to save gas since it is more efficient then i++.
2. Remove explicit 0 initialization of for loop index variable.

4. Cache array length in for loops can save gas

Impact

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack. Caching the array length in the stack saves around 3 gas per iteration.

Proof of Concept

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/Proposals/UpdateMultipleSoulConfigProposal.sol#L64

function execute() internal override returns (bool) {
    for (uint256 i = 0; i < params.length; i++) {

Tools

Remix

Recommended Mitigation Steps

Caching par = params.length and using the len instead will save gas.

5. Adding unchecked directive can save gas

Impact

For the arithmetic operations that will never over/underflow, using the unchecked directive (Solidity v0.8 has default overflow/underflow checks) can save some gas from the unnecessary internal over/underflow checks.

Proof of Concept

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/FlashGovernanceArbiter.sol#L74 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L181 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L284 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L422-L427 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Flan.sol#L107 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Limbo.sol#L225 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Limbo.sol#L327 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/UniswapHelper.sol#L232

function timeRemainingOnProposal() public view returns (uint256) {
    require(currentProposalState.decision == ProposalDecision.voting, "LimboDAO: proposal    finished.");
    uint256 elapsed = block.timestamp - currentProposalState.start;
    if (elapsed > proposalConfig.votingDuration) return 0;
    return proposalConfig.votingDuration - elapsed;
  }

Tools

Remix

Recommended Mitigation Steps

Consider using 'unchecked' where it is safe to do so. Example:

unchecked {
         amt = amount - feeAmt;
 }

6. Useless imports

Impact

Contracts FlashGovernanceArbiter.sol and FlanBackstop.sol makes no use of these imports:

Proof of Concept

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/FlashGovernanceArbiter.sol#L4 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/FlanBackstop.sol#L8

import "hardhat/console.sol";

Tools

Remix

Recommended Mitigation Steps

Consider reviewing all the unused imports and removing them to reduce the size of the contract and thus save some deployment gas.

7. Variable assigned as default

Impact

1. A variable is being assigned its default value which is unnecessary. Removing the assignment will save gas when deploying.
2. Use uint8 for burnOnTransferFee. It does not give any efficiency, actually, it is the opposite as EVM operates on default of 256-bit values so uint8 is more expensive in this case as it needs a conversion. It only gives improvements in cases where you can pack variables together, e.g. structs.

Proof of Concept

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Flan.sol#L15

uint8 public burnOnTransferFee = 0;

Tools

Remix

Recommended Mitigation Steps

1. Remove the assignment.
2. Replace uint8 with uint256

8. Change string to byteX if possible

Impact

In the ProposalFactory, declaring the description with type bytes32 can save gas.

Proof of Concept

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/ProposalFactory.sol#L11

string public description;

Tools

https://medium.com/layerx/how-to-reduce-gas-cost-in-solidity-f2e5321e0395#2a78 ##Recommended Mitigation Steps Change to:

bytes32 public description;

9. Change memory to calldata

Impact

Having an array in memory, there is an unnecessary copy from calldata to memory. In the proposed patch, this unnecessary copy is avoided and values are directly read from calldata by using calldataload(...) instead of going via calldatacopy(...), then mload(...)). Saves memory expansion cost, and cost of copying from calldata.

Proof of Concept

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L204-L205

function seed(
    address limbo,
    address flan,
    address eye,
    address proposalFactory,
    address sushiFactory,
    address uniFactory,
    address flashGoverner,
    uint256 precisionOrderOfMagnitude,
    address[] memory sushiLPs,
    address[] memory uniLPs
  ) public onlyOwner {

Tools

Remix

Recommended Mitigation Steps

Change to:

address[] calldata sushiLPs,
address[] calldata uniLPs

10. Struct layout

Impact

SecurityParameters struct in FlashGovernanceArbiter.sol can be optimized to reduce 2 storage slot ##Proof of Concept https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/FlashGovernanceArbiter.sol#L36-L41 The same in: https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/UniswapHelper.sol#L22-L33

struct SecurityParameters {
    uint8 maxGovernanceChangePerEpoch; //prevents flash governance from wrecking the incentives.
    uint256 epochSize; //only one flash governance action can happen per epoch to prevent governance DOS
    uint256 lastFlashGovernanceAct;
    uint8 changeTolerance; //1-100 maximum percentage any numeric variable can be changed through flash gov
  }

Tools

https://docs.soliditylang.org/en/v0.8.0/internals/layout_in_storage.html?highlight=Structs#layout-of-state-variables-in-storage ##Recommended Mitigation Steps

struct SecurityParameters {
    uint128 maxGovernanceChangePerEpoch; //prevents flash governance from wrecking the incentives.
    uint128 changeTolerance; //1-100 maximum percentage any numeric variable can be changed through flash gov
    uint256 epochSize; //only one flash governance action can happen per epoch to prevent governance DOS
    uint256 lastFlashGovernanceAct;
    
  }

Change the struct as suggested above.

#11. Gas optimization 2**256 - 1

Impact

when using 2**256 - 1 it takes more gas, than using type(uint256).max

Proof of Concept

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/FlanBackstop.sol#L36

IERC20(flan).approve(pyroFlan, 2**256 - 1);

Tools

Remix

Recommended Mitigation Steps

Use type(uint256).max

12. Caching variables

Impact

Some of the variable can be cached to slightly reduce gas usage.

Proof of Concept

flashGovernanceConfig.asset can be cached. https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/FlashGovernanceArbiter.sol#L66 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/FlashGovernanceArbiter.sol#L77

eye can be cached. https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L214 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L219 domainConfig.eye can be cached. https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/DAO/LimboDAO.sol#L383-L401 config.flan and config.pyroFlan can be cached. https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/FlanBackstop.sol#L75-L114 crossingConfig.behodler can be cached. https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Limbo.sol#L203-L241 flashGoverner can be cahed. https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/Limbo.sol#L380-L386 VARS.DAI can be cahed. https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/UniswapHelper.sol#L147-L157 https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/UniswapHelper.sol#L217-L235

VARS.behodler, VARS.Flan_SCX_tokenPair and VARS.flan can be cahed. https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/UniswapHelper.sol#L162-L197

Tools

Remix

Recommended Mitigation Steps

Consider caching those variable for read and make sure write back to storage.

13. Gas optimization 1 days * 365

Impact

when using 1 days * 365 it takes more gas, than using 365 days.

Proof of Concept

https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/UniswapHelper.sol#L56

uint256 constant year = (1 days * 365);

Tools

Remix

Recommended Mitigation Steps

Use uint256 constant year = 365 days;

#13. Gas optimization for normalize() function

Vulnerability details

##Impact It is possible save some gas in normalize() function. ##Proof of Concept https://github.com/code-423n4/2022-01-behodler/blob/cedb81273f6daf2ee39ec765eef5ba74f21b2c6e/contracts/FlanBackstop.sol#L121-L125

 function normalize(address token, uint256 amount) internal view returns (uint256) {
    uint256 places = config.decimalPlaces[token];
    uint256 bump = 10**(18 - places);
    return amount * bump;
  }

Tools

Remix

Recommended Mitigation Steps

Change to:

 function normalize(address token, uint256 amount) internal view returns (uint256) {
        return amount * 10** (18 - config.decimalPlaces[token]);
  }

Total : 13