Venus Prime - KrisApostolov's results

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L339-L342 https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L365 https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L398-L399

Vulnerability details

Impact

Users who have been granted an irrevocable token have the ability to mint a revocable token and accrue interest with it even if they do not possess a staked XVS balance of at least 1000 in the XVS Vault.

Proof of Concept

The protocol distinguishes between two types of tokens: Revocable and Irrevocable. Users can mint Revocable tokens themselves, and these tokens can be revoked if a user's staked XVS balance falls below 1000. In contrast, Irrevocable tokens are only minted through protocol timelock proposals and are unaffected by a user's current staking balance.

Irrevocable tokens are minted to notable users based on community voting, typically in recognition of their substantial XVS staking over an extended period.

The issue arises from how the mechanism for granting Revocable tokens to users functions. It checks the amount of staked XVS only in the xvsUpdated() function and verifies the stakedAt timestamp during token minting.

In the xvsUpdated() function:

function xvsUpdated(address user) external {
	// @audit This is the only XVS check for the user's staked XVS amount:
	uint256 totalStaked = _xvsBalanceOfUser(user);
	bool isAccountEligible = isEligible(totalStaked);
	// ...
}

In the claim() function:

function claim() external {
	// @audit	Only checks the stakedAt timestamp when minting/claiming the token:
	if (stakedAt[msg.sender] == 0) revert IneligibleToClaim();
	if (block.timestamp - stakedAt[msg.sender] < STAKING_PERIOD) revert WaitMoreTime();
	// ...
	// mint logic ...
}

With this context, let's explore the vulnerability scenario:

The assumption is that a user holding an Irrevocable token could lose it if they withdraw all their tokens or remain inactive for a specific duration.

In this scenario, a distinguished user who receives an Irrevocable token through the issue() function can exploit the situation. They can front-run the mint transaction by calling xvsUpdated() on themselves, setting their stakedAt to the current timestamp. After 90 days, they can withdraw most of their staked XVS from the vault without being revoked because of their Irrevocable token status. After some time, a timelock proposal is approved, and the community burns the user's Irrevocable token. Now, since stakedAt[user] + 90 days < block.timestamp, the user can call claim() and mint themselves a Revocable token without meeting the minimum staked XVS requirement.

This malicious user can exploit this process to continue accruing interest as if they still held an Irrevocable token.

Consider the following hardhat PoC which clearly demonstrates the vector at hand: https://gist.github.com/CrisCodesCrap/c9b509565d427095d919d2d14ec5d2be

Tools Used

Manual Review, Hardhat

Recommended Mitigation Steps

One potential mitigation strategy involves resetting the stakedAt parameter for users when minting irrevocable tokens.

delete stakedAt[users[i]];

_mint(true, users[i]);
_initializeMarkets(users[i]);

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L349-L357 https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L397-L405

Vulnerability details

Impact

Users can delay Prime token minting to earn extra interest and a larger portion of rewards for themselves.

Proof of Concept

The protocol features an issue() function designed for minting both revocable and irrevocable tokens to users via timelock proposals.

However, a potential issue arises due to a check within the _mint() function:

if (tokens[user].exists) revert IneligibleToClaim();

This check causes the entire transaction to fail if any user already possesses a token. This creates an opportunity for a user who meets the criteria of stakedAt[user] + 90 days < block.timestamp and is on the list of intended recipients for token minting to front-run the transaction and force it to revert by claiming their token.

This scenario benefits such a user because of the way the reward system operates. Reward distribution per user is determined by the sumOfMembersScore specific to the token. When more users are added, their scores contribute to the sumOfMembersScore, resulting in a smaller share of rewards per block for each user.

Consider a situation with an ongoing reward distribution cycle. In such a case, a user can simply follow the steps described above to claim additional interest from the contract before other users are included, maximising their gains.

The central assumption here is that issue() is executed via a governance proposal, preventing immediate re-execution in the same block due to the required mutation of proposal calldata.

Consider the following PoC demonstrating the issue at hand: https://gist.github.com/CrisCodesCrap/426f1e1f5b70b370c09ee9cceb8d5e01

Tools Used

Manual Review, Hardhat

Recommended Mitigation Steps

You could either adopt a pull-over-push approach or implement a check to determine if each user is eligible to call claim() independently before processing them.

for (uint256 i = 0; i < users.length; ) {
	if (stakedAt[user] + 90 days < block.timestamp) continue;

	_mint(false, users[i]);
	_initializeMarkets(users[i]);
	delete stakedAt[users[i]];

	unchecked {
		i++;
	}
}

Assessed type

DoS