Tribe Turbo contest - hyh's results

Lines of code

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

Vulnerability details

Impact

There will be an interest surplus accumulating in all the master accounting variables (totalBoosted, getTotalBoostedForVault and getTotalBoostedAgainstCollateral).

As getTotalBoostedForVault and getTotalBoostedAgainstCollateral are used in the checks against getBoostCapForVault and getBoostCapForCollateral by booster.canSafeBoostVault function on the all new boosts, this will effectively reduce these caps over time (caps figures will remain the same, but vault figures will have a surplus accumulated, so the max possible difference will shrink over time).

This process will end up blocking all the boosts as vault/collateral values be filled up to their caps with surplus vault interest payments.

In other words, the sequence of boost -> slurp -> less (with amount = everything available) leaves safe's accounting variables unchanged as net effect will be zero, while master's variables mentioned above will be increased by the cumulative interest obtained from the vault.

As such kind of sequences are part of the usual workflow, the surplus interest component will fill the master's totals up to the point when they reach the caps and prohibit any new boosts, i.e. the contract become partly stuck, not allowing new funds in

Proof of Concept

Master.onSafeLess is called after the amount was reduced to the current debt in TurboSafe.less:

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

As safe accounts for full amounts in boost/slurp/less, the safe and master accounting will diverge, with safe's one being correct, and master's one bloated by the interest payments that were accounted in slurp before:

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

Master totals are then used to validate new boosts:

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

Recommended Mitigation Steps

Consider moving master.onSafeLess(asset, vault, feiAmount) up to the VaultLessened event emission, so the full feiAmount be accounted.

Now:

emit VaultLessened(msg.sender, vault, feiAmount);

// Withdraw the specified amount of Fei from the Vault.
vault.withdraw(feiAmount, address(this), address(this));

...

// Repay Fei debt in the Turbo Fuse Pool, unless we would repay nothing.
if (feiAmount != 0) require(feiTurboCToken.repayBorrow(feiAmount) == 0, "REPAY_FAILED");

// Call the Master to allow it to update its accounting.
master.onSafeLess(asset, vault, feiAmount);

To be:

emit VaultLessened(msg.sender, vault, feiAmount);

// Call the Master to allow it to update its accounting.
master.onSafeLess(asset, vault, feiAmount);

// Withdraw the specified amount of Fei from the Vault.
vault.withdraw(feiAmount, address(this), address(this));

...

// Repay Fei debt in the Turbo Fuse Pool, unless we would repay nothing.
if (feiAmount != 0) require(feiTurboCToken.repayBorrow(feiAmount) == 0, "REPAY_FAILED");

Lines of code

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

Vulnerability details

Impact

Vault shares and collateral cTokens might be accessed with sweep if a cToken have more than one address.

This will disturb the system accounting. The tokens caller obtained will be at a loss for other safe users who will not be able to receive Fei they own per contract records.

This is a fund loss scenario, but the probability is low as such tokens are rare, so setting severity to medium

Proof of Concept

TurboSafe.sweep controls for one vault and one cTokens address, allowing for sweeping everything else:

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

Some ERC20 have more than one address they can be operated with:

https://github.com/d-xo/weird-erc20#multiple-token-addresses

Recommended Mitigation Steps

As address control is used consider providing the ability to input a list of asset addresses to prohibit operations with