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

Description

The PrelaunchPoints contract initializes the allowed token list within the constructor, enabling a predefined list of ERC20 tokens to be locked within the contract. However, the contract lacks the functionality to remove or disable tokens from the allowed list once they have been added. This could create a scenario where, due to external circumstances such as vulnerabilities discovered in a token's contract or other risk factors, the need arises to disallow further locking of that token without the ability to do so.

The immutability of the allowed tokens list means that the contract does not adhere to the principle of being prepared for future risks or changes in the token's integrity, which might jeopardize the security and integrity of the contract and the funds locked within by the users.

Example

Suppose an external ERC20 token is discovered to have a critical vulnerability that could be exploited when interacting with contracts such as PrelaunchPoints. Since this contract does not have the functionality to remove a token from the list of allowed tokens, users could unknowingly keep locking the vulnerable token, potentially leading to loss of funds or manipulative acts.

Recommendation

It is recommended to include a function that allows the contract owner to remove tokens from the isTokenAllowed mapping. This action should be protected by the onlyAuthorized modifier to ensure that only the contract's owner can execute it.

Such a function could take the following form:

function disallowToken(address _token) external onlyAuthorized {
    require(isTokenAllowed[_token], "Token is not allowed");
    isTokenAllowed[_token] = false;
    emit TokenDisallowed(_token);
}

An event would then need to be declared to log this action:

event TokenDisallowed(address indexed token);

Impact of Change

This change allows the contract to have the flexibility to respond to changes in the security and utility of different ERC20 tokens. It provides the contract owner with the necessary tools to mitigate risks related to external changes in token contracts, thus protecting users' interests.

Assessed type

Other

0xd4n1el commented 4 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 #90