code-423n4 / 2022-06-illuminate-findings

1 stars 0 forks source link

QA Report #367

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Non critical findings

Variables names

Variables can be longer and descriptive which will improve readability of the contracts

Proof of concept

All funtion parameters in the contract

    /// @param u underlying token's address, used to define the market being approved
    /// @param m maturity of the underlying token, used to define the market being approved
    /// @param r the address being approved, in this case the redeemer contract
    /// @return bool true if the approval was successful, false otherwise
    function approve(
        address u,
        uint256 m,
        address r

integer constant can be replaced with a variable

Integer constant can be stored and used in a descriptive constant variable instead of using it directly

Proof of concept

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L87

    for (uint8 i; i < 9; ) { 

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/marketplace/MarketPlace.sol#L35

    mapping(address => mapping(uint256 => address[9])) public markets;

Replace math expression with constant

The math expression for uint256 max can be replaced with type.max, or a constant MAX_UINT = type(uint).max

Proof of concept

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L112

    uint256 max = 2**256 - 1; 

Missing input validation

Input validation is missing in functions that set storage variables, variables can be accidentally set to wrong values

Proof of concept

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L61

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/marketplace/MarketPlace.sol#L53

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/redeemer/Redeemer.sol#L44

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/redeemer/Redeemer.sol#L92

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/redeemer/Redeemer.sol#L62

Missing events for critical parameter change

Functions that change critical parameter of contracts should emit events, this helps users to easily monitor changes in the protocol

Proof of concept

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L129

    function setAdmin(address a) external authorized(admin) returns (bool) {  
        admin = a;
        return true;
    }

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L137

    function setFee(uint256 f) external authorized(admin) returns (bool) {  
        feenominator = f;
        return true;
    }

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L734

    function pause(uint8 p, bool b) external authorized(admin) returns (bool) {
        paused[p] = b;
        return true;
    }

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/marketplace/MarketPlace.sol#L109

    function setAdmin(address a) external authorized(admin) returns (bool) {
        admin = a;
        return true;    
    }

Wrong comments

Proof of concept

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L153

should be set swivel address

    /// @notice sets the feenominator to the given value 
    /// @param s the address of the Swivel.sol Router
    /// @return bool true if successful
    function setSwivel(address s) external authorized(admin) returns (bool) {