Permanent token whitelisting in protocol poses exploitation risks

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L364-L366

Vulnerability details

Impact

In the event that a whitelisted token is found to be vulnerable and has been actively exploited by an attacker in the wild, the protocol needs to mitigate the issue swiftly by removing the vulnerable token from the protocol. However, the mitigation effort will be hindered by the fact that there is no way to remove a token within the protocol once it has been whitelisted. Thus, it might not be possible to stop the attacker from exploiting the vulnerable token. The protocol team would need to find a workaround to block the attack, which will introduce an unnecessary delay to the recovery process where every second counts.

Additionally, if the admin accidentally whitelisted the wrong token, there is no way to remove it.

Proof of Concept

The Loopfi protocol intends to support different LRT tokens, and the way to add a new supported token to the protocol happens by the admin calling the allowToken() function with the address of the new token that should be supported by the protocol.

 function allowToken(address _token) external onlyAuthorized {
        isTokenAllowed[_token] = true;
}

However, the protocol is missing the feature to remove a token. For example, if one of the supported LRT tokens is hacked and allows attackers to exploit Loopfi as well because it supports it, there is no way to remove this vulnerable token. Another case would be if there is an issue within one of the supported DEXs and the vulnerable token, such as price manipulation. By doing so, attackers could artificially inflate this token's price, and if they have locked this token to the protocol, they can receive much more lpETH tokens against it due to the manipulated price. Eventually stealing from other users and the protocol, as this will cause the lpETH token to be also affected, this can lead to drop of price, liquidation on protocols which supports it as collateral and so on. And If one of these scenarios becomes real, there is no way to remove/block usage of the vulnerable token.

Something similar happened when Luna imploded. Due to the missing failsafe mechanisms in place, this allowed malicious actors to drain two protocols - one for \$13.5M and the second one for \$8.3M respectively due to Chainlink oracle returning the wrong price, and as mentioned, there were no failsafe mechanisms. Link to the exploit: https://rekt.news/venus-blizz-rekt/

Tools Used

VSCode

Recommended Mitigation Steps

Consider implementing an additional function to disallow tokens from the whitelist, enabling the swift removal of vulnerable tokens if necessary.

 function disallowToken(address _token) external onlyAuthorized {
        isTokenAllowed[_token] = false;
}

Additionally, incorporate checks to determine if operations for withdrawing and claiming can proceed with this token, similar to those in the _processLock() function:

    if (!isTokenAllowed[_token]) {
        revert TokenNotAllowed();
    }
    perfrom the operation

Assessed type

Other

[0xd4n1el]

Disallowing tokens poses a critical risk in case of malicious owner, since it has the power to withdraw non allowed tokens. Also, there is no risk for users or for the team on allowing malicious tokens, or at least not proven in this submission.

[c4-judge]

koolexcrypto marked the issue as duplicate of #90

[radin200]

I believe this is not an actual issue but a proposal for improvement of the protocol. I do not think this should result in a valid finding. In the extreme conditions that a wrong token is whitelisted by the trusted owner. Which itself does invalidate the issue given the owner will not make mistakes by readMe.

[c4-judge]

koolexcrypto marked the issue as unsatisfactory: Invalid