howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

The PrelaunchPoints contract allows the owner to add support for new tokens to be locked using the allowToken function. However, there is no corresponding function to disallow or remove a previously allowed token. This could lead to a situation where a malicious or faulty token remains locked in the contract indefinitely, potentially causing loss of funds or unexpected behavior.

Issue Description

The allowToken function is used by the owner to add a new token to the list of allowed tokens that can be locked in the PrelaunchPoints contract:

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

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

Once a token is allowed, users can call the lock or lockFor functions to lock that token in the contract:

function lock(address _token, uint256 _amount, bytes32 _referral) external {
    if (_token == ETH) {
        revert InvalidToken();
    }
    _processLock(_token, _amount, msg.sender, _referral);
}

function lockFor(address _token, uint256 _amount, address _for, bytes32 _referral) external {
    if (_token == ETH) {
        revert InvalidToken();
    }
    _processLock(_token, _amount, _for, _referral);
}

The issue is that once a token is allowed, there is no way to disallow it. If a token that was previously considered safe turns out to be malicious or faulty (for example, if it has a hidden mint function that allows arbitrary inflation), the owner has no way to prevent further locking of that token.

This could lead to a situation where users continue to lock a bad token, potentially losing funds or causing unexpected behavior in the contract. Even if the owner detects the issue, they cannot stop the inflow of the bad token without deploying a new version of the contract.

Furthermore, if a significant amount of a malicious token gets locked before the issue is detected, those tokens will remain stuck in the contract indefinitely. This could impact the contract's ability to function properly and could also cause financial loss for users who locked the bad token.

Proof of Concept

Consider a scenario where the owner allows a new token MaliciousToken that seems safe at first:

MaliciousToken maliciousToken = new MaliciousToken();
prelaunchPoints.allowToken(address(maliciousToken));

Users start locking MaliciousToken in the PrelaunchPoints contract:

prelaunchPoints.lock(address(maliciousToken), 1000, bytes32(0));

Later, it's discovered that MaliciousToken has a hidden mint function that allows arbitrary inflation. However, by this point, a significant amount of MaliciousToken has already been locked.

The owner has no way to disallow further locking of MaliciousToken. Users who are unaware of the issue continue to lock the token, potentially losing funds.

Moreover, the MaliciousToken that were locked before the issue was detected remain stuck in the PrelaunchPoints contract indefinitely.

Tools Used

Manual code review

Recommended Mitigation Steps

Add a disallowToken function that allows the owner to remove a token from the allowed list:

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

With this function, if a token is found to be malicious or faulty, the owner can immediately prevent further locking of that token.

Assessed type

ERC20

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 changed the severity to QA (Quality Assurance)