Skip to content
This repository has been archived by the owner on Jan 12, 2025. It is now read-only.

Smacaud - Users can game the system with position.initialLockDuration #426

Closed
sherlock-admin4 opened this issue Jul 15, 2024 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin4
Copy link

sherlock-admin4 commented Jul 15, 2024

Smacaud

High

Users can game the system with position.initialLockDuration

Summary

The MlumStaking contract, in addToPostion function, calculates an average duration (avgDuration) when users add to their staking positions, but fails to use this avgDuration for determining the lock multiplier. Instead, it continues to use the initial lock duration which could lead to user getting more rewards than they should

Vulnerability Detail

In the addToPosition function, it calculates rewards based on the initial lock duration. Users can exploit this by creating a long initial lock, then adding small amounts with minimal lock duration.

Let's consider this example

User A:

Stakes 1000 tokens for 365 days

lockMultiplier = getMultiplierByLockDuration(365 days) = _maxLockMultiplier = 20000

amountWithMultiplier = 1000 * (20000 + 10000) / 10000 = 3000

User B:

Initially stakes 200 tokens for 250 days

lockMultiplier = getMultiplierByLockDuration(250 days) = (20000 * 250) / 365 ≈ 13698

amountWithMultiplier = 200 * (13698 + 10000) / 10000 ≈ 474

Adds 800 tokens after 115 days

remainingLockTime = 250 - 115 = 135 days
avgDuration = (135 * 200 + 250 * 800) / 1000 = 227 days

position.lockDuration is updated to 227 days

However, lockMultiplier remains based on initialLockDuration (250 days)

New amountWithMultiplier = 1000 * (13698 + 10000) / 10000 = 2370

The main point is when User B adds to their position, their lockMultiplier doesn't change. It's still based on the initial 250-day lock, even though their average lock duration has decreased to 227 days.

This be could seen as gaming the system because in the example, User B maintains a higher multiplier (based on 250 days) for their entire stake, even though a large portion (800 tokens) is only locked for 135 days. Compared to a user who would stake 1000 tokens for 227 days from the start, User B gets a higher multiplier and thus more rewards.

Impact

Users who add to their positions with shorter remaining lock times still benefit from the full multiplier of their initial longer lock.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MlumStaking.sol#L397

https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/MlumStaking.sol#L656

Tool used

Manual Review

Recommendation

Update lockMultiplier to use the avgDuration which is set as position.lockDuration = avgDuration;

position.lockMultiplier = getMultiplierByLockDuration(position.lockDuration);

Duplicate of #138

@github-actions github-actions bot added duplicate Medium A Medium severity issue. labels Jul 21, 2024
@sherlock-admin4 sherlock-admin4 added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Jul 22, 2024
@sherlock-admin4 sherlock-admin4 changed the title Square Rusty Gazelle - Users can game the system with position.initialLockDuration Smacaud - Users can game the system with position.initialLockDuration Jul 29, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Jul 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant