QA Report

[code423n4]

C4 finding submitted: (risk = 1 (Low Risk)) Slurp function may revert when it shouldn't

Lines of code

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

Vulnerability details

Impact

slurp function checking for a non-zero getTotalFeiBoostedForVault[vault] value may cause problems.

Proof of Concept

1-Alice boosts vault V with 100 tokens. getTotalFeiBoostedForVault[vault] = 100 2-After sometime, vault accrues 1 interest. 3-Alice lesses vault V for 100 tokens. getTotalFeiBoostedForVault[vault] = 0 4-When slurp is called, it will revert although vault has 1 token.

Tools Used

Manual analysis

Recommended Mitigation Steps

Check against interestEarned instead. First 2 lines of the function can be changed as:

    // Compute the amount of Fei interest the Safe generated by boosting the Vault.
    uint256 interestEarned = vault.assetsOf(address(this)) - getTotalFeiBoostedForVault[vault];

// Ensure the Safe has earned interest in the Vault.
    require(interestEarned != 0, "NO_FEI_BOOSTED");

C4 finding submitted: (risk = 1 (Low Risk)) Non-zero recipient address checks are missing.

Lines of code

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L306 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L335 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L318 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/modules/TurboGibber.sol#L74 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/modules/TurboGibber.sol#L98

Vulnerability details

Impact

If the recipient address is zero, tokens will be burnt accidentially. It is a good practice to check the address is non-zero.

Proof of Concept

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L306 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L335 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L318 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/modules/TurboGibber.sol#L74 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/modules/TurboGibber.sol#L98

Tools Used

Manual analysis

Recommended Mitigation Steps

Add a non-zero address check before token transfers require(to != address(0), "Zero address");

C4 finding submitted: (risk = 0 (Non-critical)) There are 2 sweep functions and TokenSweeped events

Lines of code

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L300 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L306 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L312 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L318

Vulnerability details

Impact

It is not a good practice to use exact same function names for 2 different functions. Also the created events are exactly the same, so it would not be possible to distinguish master sweeps events from safe sweep events.

Proof of Concept

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L300 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L306 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L312 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L318

Tools Used

Manual analysis

Recommended Mitigation Steps

Rename the sweep functions and their events, so that they can be distinguished.

[GalloDaSballo]

Revert on nonZero -> Pretty good finding, may be ideal to slurp on less Address zero check -> Agree by convention Sweep convention -> Personally disagree as the function does the same thing, the tweaks are for context

I appreciated that the warden put links for the findings, making mitigation easier.

I think formatting could have been done a lot better, especially titles, linking to sections and overall making the report easier to scan through.

[GalloDaSballo]

4/10