Tribe Turbo contest - catchup's results

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.

C4 finding submitted: (Gas) Missing non-zero amount checks

Lines of code

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/TurboSafe.sol#L171 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L210 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L306

Vulnerability details

Impact

Functions do not check if the amount is non-zero. A zero amount would result in unnecessary transaction and gas usage.

Proof of Concept

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/TurboSafe.sol#L171 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L210 https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L306

Tools Used

Manual analysis

Recommended Mitigation Steps

Check if the amount is non-zero

C4 finding submitted: (Gas) Changing the order in boost function can save gas

Lines of code

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

Vulnerability details

Impact

require(feiTurboCToken.borrow(feiAmount) == 0, "BORROW_FAILED"); can be called just after master.onSafeBoost(asset, vault, feiAmount); In case the borrow fails and reverts, the intermediate steps would be avoided.

Proof of Concept

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

Tools Used

Manual analysis

Recommended Mitigation Steps

Change the order as: // Ensure the Vault accepts Fei asset. require(vault.asset() == fei, "NOT_FEI");

    // Call the Master where it will do extra validation
    // and update it's total count of funds used for boosting.
    master.onSafeBoost(asset, vault, feiAmount);

    // Borrow the Fei amount from the Fei cToken in the Turbo Fuse Pool.
    require(feiTurboCToken.borrow(feiAmount) == 0, "BORROW_FAILED");

.
.