search

code-423n4 / 2024-05-munchables-validation

0 stars 0 forks source link

Inaccurate Calculation of Weighted Value When Lock Duration Expires but Tokens Remain Unlocked #126

Open c4-bot-9 opened 4 months ago

c4-bot-9 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L461

Vulnerability details

Impact

The current implementation inaccurately calculates the weighted value of locked tokens when the lock duration has expired but the user has not yet unlocked them. This discrepancy can lead to misleading representations of the user's locked token holdings, affecting decision-making processes based on the reported values.

the getLockedWeightedValue function checks to ensure that the player quantity is > 0 as below

lockedTokens[_player][configuredTokenContracts[i]].quantity >
                0 &&
                configuredTokens[configuredTokenContracts[i]].active

the issue with that is before unlocking the tokens users who locked have a quantity > 0 even if the lock duration is passed.

Proof of Concept (PoC):

Suppose a user locks tokens for a specific duration. Once the lock duration expires, the user does not immediately unlock the tokens. The system calculates the weighted value of the locked tokens using the getLockedWeightedValue function. Despite the lock duration expiring, the system still considers the tokens as locked and includes them in the calculation. As a result, the reported weighted value inaccurately includes these tokens, leading to misleading representations of the user's actual locked token holdings.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a mechanism to accurately track and update the status of locked tokens once the lock duration expires. Consider automatically adjusting the locked status of tokens when the lock duration ends, ensuring that only actively locked tokens are included in weighted value calculations.

Assessed type

Invalid Validation

pratokko commented 3 months ago

Hello, I would like to comment on this, The current getLockedWeightedValue function inaccurately includes tokens in its calculation if the lock duration has expired but the user has not unlocked them. This results in misleading representations of a user's locked token holdings.

Consider a user locking tokens for a set duration. After the lock duration expires, the user doesn't immediately unlock the tokens. The getLockedWeightedValue function still considers these tokens as locked, leading to inaccurate weighted value calculations.

I humbly request this to be considered a valid medium

alex-ppg commented 3 months ago

Hey @pratokko, thanks again for your input! The weighted value is solely utilized to distribute Schnibble rewards which abide by a distinct tracking system. That tracking system maintains its own timestamps in relation to when a claim last occurred, and whether the lock has expired or not has no bearing on those rewards as they are tracked per second. Based on this, an expired lock should continue earning Schnibble rewards as long as it remains in the LockManager which is the present behavior.