code-423n4 / 2022-02-tribe-turbo-findings

1 stars 0 forks source link

QA Report #36

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L72 Add validity or empty check before setting booster

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L93 Add validity or empty check before setting clerk

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L114 Add validity or empty check before setting authority

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L180

Below code can be optimized further // Prepare a users array to whitelist the Safe. address[] memory users = new address; users[0] = address(safe);

    // Prepare an enabled array to whitelist the Safe.
    bool[] memory enabled = new bool[](1);
    enabled[0] = true;

All above can be replaced with address[1] memory users = [address(safe)];

    // Prepare an enabled array to whitelist the Safe.
    bool[] memory enabled = [true];

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L168 Flawed logic

safe = new TurboSafe(msg.sender, defaultSafeAuthority, asset);
safes.push(safe);
unchecked {
    // Get the index/id of the new Safe.
    // Cannot underflow, we just pushed to it.
    id = safes.length - 1;
}
// Store the id/index of the new Safe.
getSafeId[safe] = id;

In above code, when first safe is pushed to safes, safes.length will be 1. So id will 0. If TurBoSafe() creation fails, further logic will throw error. You can remove the unchcked block and replace with below getSafeId[safe] = safes.length - 1;

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L177 It's better to emit after all processing is done

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L323 It's better to emit after safeTransfer

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L197 emit FeiDeposited() event

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L317 Raise the event after token sweep is approved and completed

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboRouter.sol#L74 If not called internally, function should be declared external. The function deposit is not called inside the contract.

GalloDaSballo commented 2 years ago

Report has plenty of small findings, formatting could have been done better, but ultimately does cover the basics. Well done by the warden

GalloDaSballo commented 2 years ago

6/10

GalloDaSballo commented 2 years ago

After re review I think 5/10 is more appropriate. I liked the optimization, everything else is the usual events and validation finding