Lack of Functionality to Disallow Tokens from the allowed list

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L97

Vulnerability details

Impact

Currently, the protocol allows the addition of tokens to the list of allowed tokens dynamically but lacks a mechanism to disallow tokens once they have been added. This poses several risks and limitations:

Allowing tokens dynamically increases the risk of malicious or incompatible tokens being added to the protocol, potentially leading to security vulnerabilities or protocol disruptions.

Tokens that are incompatible with the protocol or have faulty contracts may disrupt the operation of the protocol if not properly managed.

Users' assets may be at risk if a token with a faulty contract or malicious behavior is allowed within the protocol. Without the ability to remove such tokens, users may face potential loss or exploitation.

Proof of Concept

At the moment tokens can be added to this contract during deployment in the constructor of using the allowToken function.

However, the contract currently lacks a function to disallow tokens. The allowToken function allows tokens to be added dynamically, while no corresponding function exists to remove them. Additionally, the constructor initializes the list of allowed tokens but does not provide a way to modify or remove them.

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

constructor(address _exchangeProxy, address _wethAddress, address[] memory _allowedTokens) {
    ...
    for (uint256 i = 0; i < _allowedTokens.length; i++) {
        isTokenAllowed[_allowedTokens[i]] = true;
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a function to disallow tokens, allowing the owner or authorized users to remove tokens from the list of allowed tokens. This would mitigate risks, enhance protocol stability, and protect users' assets.

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

Assessed type

Context

[0xd4n1el]

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

[c4-judge]

koolexcrypto marked the issue as duplicate of #98

[c4-judge]

koolexcrypto marked the issue as duplicate of #90

[c4-judge]

koolexcrypto marked the issue as unsatisfactory: Invalid

[pratokko]

Hello, I would like to comment on this, based on sponsor claims

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

but this is not the case if tokens can just be added and lack a functionality to be removed from the list, there is a security risk. if the tokens cannot be removed then It means users will still be able to interact with the token as the check for allowed tokens will always pass. I humbly request this to be a valid medium. Thank you

[koolexcrypto]

Hi @pratokko

disallowing a token will allow the owner to withdraw funds from the contract. This is the security risk the sponsor meant.

There is no risk from not having disallowing tokens. If there is a mistake, you can simply just re-deploy the same code. It is an admin action.