QA report

[code423n4]

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

[Joeysantoro]

This is an extremely niche edge case. I'm inclined to think multiple address tokens will not be approved for Turbo. Will contest to low and leave as acknowledged unless @transmissions11 has other ideas

[transmissions11]

Collateral cTokens having multiple addresses is out of scope because we control the Turbo pool and would never make such a token.

As for Vault shares in theory it could be an issue but once again we can vet Vaults to ensure they're not unsafe like this.

[GalloDaSballo]

I believe the finding to have merit and appreciate the warden's submission.

However I have to agree with the sponsor in that there is no reason to deploy a second cToken with the same underlying and if that were to happen remediation would be very quick.

So I'll mark the finding as valid but non-critical as it technically is correct, could happen but ultimately is not a vulnerability

[GalloDaSballo]

Will judge as a separate report 3/10

[GalloDaSballo]

Crazy how with the TUSD Compound Disclosure this finding takes a different meaning.

With the additional information I have :
https://medium.com/chainsecurity/trueusd-compound-vulnerability-bc5b696d29e2

I believe the finding is of Low Severity and will re-rate to 5/10 as a single QA report.

Ultimately I believe this can happen and if that were the case the finding could have a high impact, however the likelihood is extremely low, and would require the sponsor to mistakenly add a token with multiple address.

[CloudEllie]

Since this issue was downgraded to a QA level, and the warden did not submit a separate QA report, we've renamed this one to "QA report" for consistency.

The original title, for the record, was "TurboSafe.sweep can't control several addresses per token."