Venus Prime - Satyam_Sharma's results

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L331-L359

Vulnerability details

Impact

user can still claim revocable token even after upgraded to irrevocable token

Proof of Concept

Prime.issue, issue prime token to user by checking if the token is irrevocable or revocable, https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L331C5-L359C6

 if (isIrrevocable) {
            for (uint256 i = 0; i < users.length; ) {
                Token storage userToken = tokens[users[i]];
                if (userToken.exists && !userToken.isIrrevocable) {
                    _upgrade(users[i]);
                } else {
                    _mint(true, users[i]);
                    _initializeMarkets(users[i]);
                }

                unchecked {
                    i++;
                }

if token issued is irrevocable it runs a for loop which saves user prime token's metadata to userToken after that it checks for the user token metdata that if the token exists and if the user token is revocable, if (userToken.exists && !userToken.isIrrevocable) than it calls to _upgrade which upgrade the user token to irrevocable by incrementing totalIrrevocable and decrementing totalRevocable and if this condition fails the issue function calls to _mint and _initializeMarkets for irrevocable tokens,

further if the issued token is not irrevocable it calls _mint and _initializeMarkets after that it deletes the stakedAt mapping which tracks the claiming of tokens,

else {
            for (uint256 i = 0; i < users.length; ) {
                _mint(false, users[i]);
                _initializeMarkets(users[i]);
                delete stakedAt[users[i]];

                unchecked {
                    i++;
                }
            }
        }

The issue arises when the token is upgraded to irrevocable, but the stakedAt mapping, which tracks when users can claim tokens, is not deleted. This means that even though the token has been upgraded to irrevocable, users can still claim revocable tokens if they were eligible before the upgrade.

    if (userToken.exists && !userToken.isIrrevocable) {
                    _upgrade(users[i]);

Tools Used

manual

Recommended Mitigation Steps

delete the claiming period for revocable token after the token get upgraded to irrevocable token, delete stakedAt[users[i]];

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L208-L226

Vulnerability details

Impact

Prime.updateScores the for loop will never get incremented if isScoreUpdated is true, due to the continue statement that will lead to run for loop infinite number of times.

Proof of Concept

Prime.updateScores updates the score for every user, who has registered prime token in the contract under this function it checks if the user score updated for not as if (isScoreUpdated[nextScoreUpdateRoundId][user]) continue; and if isScoreUpdated = true for any user conditional "continue" statement will triggered and the code continues to next iteration without incrementing "i" hence the next loop is executed for the same index resulting in an infinite loop which reverts due to txn gas limit.

Prime.sol#L208-L226

       if (isScoreUpdated[nextScoreUpdateRoundId][user]) continue;

            address[] storage _allMarkets = allMarkets;
            for (uint256 j = 0; j < _allMarkets.length; ) {
                address market = _allMarkets[j];
                _executeBoost(user, market);
                _updateScore(user, market);

                unchecked {
                    j++;
                }
            }

            pendingScoreUpdates--;
            isScoreUpdated[nextScoreUpdateRoundId][user] = true;

            unchecked {
                i++;
            }

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L208C9-L226C14

Tools Used

manual

Recommended Mitigation Steps

The solution for this issue would be to either manually increment the "i" when we "continue" or to simply use the default "for" loop syntax without the gas opt

Assessed type

Loop