Calculating points is susceptible to precision loss due to division before multiplication

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1155

Vulnerability details

Impact

Points may be lost due to division before multiplication precision issues.

Proof of Concept

NeoTokyoStaker.sol#L1155

// Update caller staking information and asset data.
PoolData storage pool = _pools[AssetType.LP];
unchecked {
    uint256 points = amount * 100 / 1e18 * timelockMultiplier / _DIVISOR;

    // Update the caller's LP token stake.
    stakerLPPosition[msg.sender].timelockEndTime =
        block.timestamp + timelockDuration;
    stakerLPPosition[msg.sender].amount += amount;
    stakerLPPosition[msg.sender].points += points;

    // Update the pool point weights for rewards.
    pool.totalPoints += points;
}

Tools Used

Manual review

Recommended Mitigation Steps

Consider multiplying before dividing

-     uint256 points = amount * 100 / 1e18 * timelockMultiplier / _DIVISOR;
+     uint256 points = amount * 100 * timelockMultiplier / (1e18 * _DIVISOR);

[c4-judge]

hansfriese marked the issue as satisfactory

[c4-judge]

hansfriese marked the issue as duplicate of #304

[c4-judge]

hansfriese marked the issue as duplicate of #261

[c4-judge]

hansfriese marked the issue as not a duplicate

[c4-judge]

hansfriese marked the issue as duplicate of #304

[c4-judge]

hansfriese changed the severity to QA (Quality Assurance)

[c4-judge]

hansfriese marked the issue as grade-b