QA Report

[code423n4]

C4-001 : Deprecated safeApprove() function

Impact - LOW

Detailed description of the impact of this finding.

Using this deprecated function can lead to unintended reverts and potentially the locking of funds. A deeper discussion on the deprecation of this function is in OZ issue #2219 (OpenZeppelin/openzeppelin-contracts#2219). The OpenZeppelin ERC20 safeApprove() function has been deprecated, as seen in the comments of the OpenZeppelin code.

Proof of Concept

1. Navigate to the following contract functions.

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/TurboSafe.sol#L89

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/modules/TurboGibber.sol#L55

Tools Used

Code Review

Recommended Mitigation Steps

As suggested by the OpenZeppelin comment, replace safeApprove() with safeIncreaseAllowance() or safeDecreaseAllowance() instead.

C4-002 : The Contract Should Approve(0) first

Impact - LOW

Some tokens (like USDT L199) do not work when changing the allowance from an existing non-zero allowance value.
They must first be approved by zero and then the actual allowance must be approved.

IERC20(token).approve(address(operator), 0);
IERC20(token).approve(address(operator), amount);

Proof of Concept

1. Navigate to the following contract functions.

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/TurboSafe.sol#L89

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/modules/TurboGibber.sol#L55

Tools Used

None

Recommended Mitigation Steps

Approve with a zero amount first before setting the actual amount.

C4-003 : Lack Of Return Value Check On the Oracle

Impact - LOW

During the code review, It has been seen that oracle return value has not been checked on the function. If oracle is returned price as a 0, borrowLimit will be zero.

Proof of Concept

1. Navigate to the following contract functions.

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/modules/TurboSavior.sol#L121

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/modules/TurboSavior.sol#L125

Tools Used

None

Recommended Mitigation Steps

Consider to add return value check on the oracle function.

C4-004 : Missing zero-address check in constructors and the setter functions

Impact - LOW

Missing checks for zero-addresses may lead to infunctional protocol, if the variable addresses are updated incorrectly.

Proof of Concept

1. Navigate to the following contract functions.

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/TurboSafe.sol#L52

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/modules/TurboGibber.sol#L45

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/TurboMaster.sol#L47

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/TurboMaster.sol#L46

Tools Used

Code Review

Recommended Mitigation Steps

Consider adding zero-address checks in the discussed constructors:
require(newAddr != address(0));.

C4-005 : Incompatibility With Rebasing/Deflationary/Inflationary tokens

Impact - LOW

The protocol do not appear to support rebasing/deflationary/inflationary tokens whose balance changes during transfers or over time. The necessary checks include at least verifying the amount of tokens transferred to contracts before and after the actual transfer to infer any fees/interest.

Proof of Concept

1. Navigate to the following contract.

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/TurboSafe.sol#L342

Tools Used

Manual Code Review

Recommended Mitigation Steps

• Ensure that to check previous balance/after balance equals to amount for any rebasing/inflation/deflation
• Add support in contracts for such tokens before accepting user-supplied tokens
• Consider supporting deflationary / rebasing / etc tokens by extra checking the balances before/after or strictly inform your users not to use such tokens if they don't want to lose them.

[Joeysantoro]

Disputing C-003, the oracle has to be valid to add an asset to the turbo pool. Ack but not fixing rest

[GalloDaSballo]

Approves -> Agree
SafeApprove(0) -> Agree (esp if not known), but FEI is known tbf

C-003 agree with sponsor as Oracle is from Fuse and already validated.
Zero checks -> Agree by convention

Inflationary tokens -> Out of scope per other finding, but we'll ack anyway

Overall a pretty good report with a couple of nice findings

[GalloDaSballo]

4/10