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

1 stars 0 forks source link

QA Report #406

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

LOW

Missing address(0) check when setting immutable address variables.

The gas cost for this check is very minimal compared to the gas cost of redeployment if an address(0) is accidentally passed in.

1. File: Redeemer.sol#L54-55

    pendleAddr = p;
    tempusAddr = t;

2. File: MarketPlace.sol#L55-56

    redeemer = r;
    lender = l;

3. File: Lender.sol#L68-69

    pendleAddr = p;
    tempusAddr = t;

QA

Critical changes should use two-step procedure

1. File: Redeemer.sol#L62-65

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

2. File: Lender.sol#L129-132

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

3. File: MarketPlace.sol#L109-112

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

Variable names should be meaningful and give an idea of its purpose.

This is an issue persistent throughout almost the entire codebase.

Using a single letter as a variable name makes reading and understanding much more difficult.

Multiple files should not have the same name

1. File: MarketPlace.sol

2. File: MarketPlace.sol

Incorect @notice Natspec comment

This function sets swivelAddr not feenominator

1. File: Lender.sol#L153-159

    /// @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) {
        swivelAddr = s;
        return true;
    }