code-423n4 / 2024-05-loop-findings

4 stars 4 forks source link

Permanent Whitelisting in PrelaunchPoints Contract Poses Security Risks #93

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L364

Vulnerability details

Impact

The inability to set isTokenAllowed to false means that once a token is whitelisted, it cannot be removed. This presents several risks e.g If a whitelisted token is found to have vulnerabilities or is compromised, it cannot be removed, potentially endangering the assets locked in the contract and it cannot respond to issues with whitelisted tokens, such as hacks.

Proof of Concept

The contract lacks a function to update the isTokenAllowed mapping to false. The only related function, allowToken, can set a token's status to true but not the opposite.nshots, logs, or any other relevant proof that illustrates the concept.

Tools Used

Manual

Recommended Mitigation Steps

+       function allowToken(address _token, bool _allowed) external onlyAuthorized {
+        isTokenAllowed[_token] = _allowed;
    }

This way function allowToken can be used to both white-list and delist tokens. Note: this is only suggested because its said admin is trusted. As this could be used to withdraw funds from the contract by admin via function recoverERC20.

Assessed type

Other

0xd4n1el commented 3 months ago

This poses a security risk in case of malicious owner, since disallowedTokens can be withdrawn by owner

c4-judge commented 3 months ago

koolexcrypto marked the issue as duplicate of #98

c4-judge commented 3 months ago

koolexcrypto marked the issue as duplicate of #90

c4-judge commented 3 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid