Protocol can break for a token with a proxy and implementation contract (like `TUSD`)

[codehawks-bot]

Protocol can break for a token with a proxy and implementation contract (like TUSD)

Severity

High Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L112

Summary

Tokens whose code and logic can be changed in future can break the protocol and lock user funds.

Vulnerability Details

For a token like TUSD (supported by Chainlink TUSD/USD price feed), which has a proxy and implementation contract, if the implementation behind the proxy is changed, it can introduce features which break the protocol, like choosing to not return a bool on transfer(), or changing the balance over time like a rebasing token.

Impact

Protocol may break in future for this collateral and block user funds deposited as collateral. Also can cause bad loans to be present with no way to liquidate them.

Tools Used

Manual review

Recommendations

• Developers integrating with upgradable tokens should consider introducing logic that will freeze interactions with the token in question if an upgrade is detected. (e.g. the TUSD adapter used by MakerDAO).
• OR have a token whitelist which does not allow such tokens.

[kosedogus]

This finding is not about tokens that change wallet's balance according to some conditions (rebasing and airdrop). Duplicates of this finding is about rebasing tokens and airdrop tokens but this finding is different than them. I recommend making this finding unique (if it is acceptable) and choose new selected finding for rebasing issue.

[PatrickAlphaC]

ERC20s are such a security disaster.

[PatrickAlphaC]

checking with Hans

[hans-cyfrin]

We can't accept all weird ERC20 types as separate medium issues. Fee-on-transfer and rebasing tokens are generally accepted in other competitions as well. I still think the current judging for this one is reasonable and would group all weird-erc20 issues into one in the future.