Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Albort - The investment round and KYC address investment limits may be exceeded. #303

Open
sherlock-admin2 opened this issue Nov 17, 2024 · 0 comments

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Nov 17, 2024

Albort

High

The investment round and KYC address investment limits may be exceeded.

Summary

https://github.com/sherlock-audit/2024-11-vvv-exchange-update/blob/1791f41b310489aaa66de349ef1b9e4bd331f14b/vvv-platform-smart-contracts/contracts/vc/VVVVCInvestmentLedger.sol#L175
In the current invest function, the logic to check whether the investment exceeds individual or round limits may fail under concurrent transactions, causing the total investment to surpass the set limits.

Root Cause

In the invest function, the logic for checking investment limits is as follows:

// Check if the investment exceeds the individual or round limits
if (
    postFeeStableAmountEquivalent > _params.kycAddressAllocation - kycAddressInvestedThisRound ||
    postFeeStableAmountEquivalent > _params.investmentRoundLimit - totalInvestedThisRound
) {
    revert ExceedsAllocation();
}

This check assumes that kycAddressInvestedThisRound and totalInvestedThisRound are accurate for the current transaction. However, in scenarios where multiple transactions occur simultaneously (e.g., when multiple users submit transactions almost at the same time or a single user rapidly submits multiple transactions), the following issues may arise:

1.Race Condition: Two or more transactions may see the same totalInvestedThisRound and kycAddressInvestedThisRound values at the time of the check, because the state is only updated after each transaction is executed.
2.Exceeding Investment Limits: If multiple transactions pass the limit checks simultaneously, the cumulative investment amount after all transactions are executed may exceed the specified limit.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Assume the following:

investmentRoundLimit is 10,000.
The current totalInvestedThisRound is 9,000.
Two users each attempt to invest 1,500 at the same time.

During each transaction:

•	When the limit check is performed, totalInvestedThisRound is 9,000.
•	The available amount to invest is calculated as: 10,000 - 9,000 = 1,000.
•	Since 1,500 > 1,000, the check fails, and both transactions are rejected.

However, if each user tries to invest 1,000:

•	At the time of the check, totalInvestedThisRound is still 9,000.
•	The available amount to invest is: 10,000 - 9,000 = 1,000.
•	Since 1,000 <= 1,000, both checks pass, and both transactions are executed.
•	After both transactions are completed, totalInvestedThisRound becomes 11,000, exceeding the limit.

Impact

No response

PoC

No response

Mitigation

No response

@sherlock-admin3 sherlock-admin3 changed the title Abundant Mauve Rook - The investment round and KYC address investment limits may be exceeded. Albort - The investment round and KYC address investment limits may be exceeded. Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant