Skip to content

Latest commit

 

History

History
125 lines (87 loc) · 5.34 KB

106.md

File metadata and controls

125 lines (87 loc) · 5.34 KB

Massive Frost Yak

Medium

Precision Loss in Time-Based Calculations Due to Integer Division

Summary

Integer division in time calculations will cause a precision loss for users as the contract will round down timestamps, potentially leading to shorter lock durations or miscalculated reward periods.

Root Cause

In checkIncreaseUnlockTime and notifyRewardAmounts, the calculation (block.timestamp / 1 weeks) * 1 weeks performs integer division before multiplication, which causes precision loss.

Internal pre-conditions

  1. The contract needs to be deployed and initialized.
  2. Users need to interact with functions that rely on these time calculations, such as checkIncreaseUnlockTime or notifyRewardAmounts.

External pre-conditions

None

Attack Path

  1. A user calls a function that uses checkIncreaseUnlockTime, such as vote or poke.
  2. The function calculates the unlock time using (block.timestamp / 1 weeks) * 1 weeks.
  3. Due to integer division, the calculated time is rounded down to the nearest week.
  4. This results in a shorter lock duration than intended.

Let's consider an example with a non-aligned duration:

Assume lockDuration is 1,234,567 seconds (about 14 days, 6 hours, 56 minutes, and 7 seconds)

$$\begin{align} \text{block.timestamp} + \text{lockDuration} &= 1697155200 + 1234567 \\\ &= 1698389767 \text{ (Friday, October 27, 2023 6:56:07 AM UTC)} \\[10pt] 1698389767 \div 604800 &= 2808 \text{ (weeks since the Unix epoch)} \\[10pt] 2808 \times 604800 &= 1698364800 \text{ (Friday, October 27, 2023 12:00:00 AM UTC)} \end{align}$$

In this case, you can see that the unlock time has been rounded down to the nearest week boundary. The extra 6 hours, 56 minutes, and 7 seconds were truncated, ensuring the unlock happens at the start of the week.

Similarly for notifyRewardAmounts:

  1. An operator calls notifyRewardAmounts to distribute rewards.
  2. The function calculates the current period using (block.timestamp / 1 weeks) * 1 weeks.
  3. Due to integer division, the calculated period is rounded down to the nearest week.
  4. This may result in incorrect period calculations and potentially affect reward distributions.

The same rounding down effect occurs in this function, potentially leading to misaligned reward periods.

Impact

Users may experience shorter lock durations than intended, potentially reducing their voting power or rewards. The protocol may also face issues with reward period calculations, leading to inconsistencies in reward distributions. As demonstrated in the example, users could lose up to 6 days, 23 hours, 59 minutes, and 59 seconds of lock time in the worst case. While the impact per transaction might seem small, it could accumulate over time and affect many users, especially those who frequently interact with the contract.

PoC

To Test the POC follow the below steps:

# To run the following POC paste the below code in BugTest.sol file
# Type the following command
forge test --mt test_PrecisionLoss -vvvv

Here is the POC demonstrating the precision loss using stateless fuzzing:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.10;
import {Test} from "forge-std/Test.sol";

contract BugTest is Test{
    function setUp() public {}

    function test_PrecisionLoss(uint256 time) public {
        // Min: Block.timestamp is 1728727271 => IST 2024-10-12 03:31:00 PM
        // Max: 100_000 days = 8640000000 seconds
        uint BoundedTime = bound(time, 1728727271, 8640000000);
        uint256 currentPeriod = (time / 1 weeks) * 1 weeks;
        assertEq(currentPeriod, BoundedTime, "Precision Loss");
    }
}

Here is the stack trace of the failed test:

Ran 1 test for test/BugTest.sol:BugTest
[FAIL. Reason: Precision Loss: 3424377600 != 3424681274; counterexample: calldata=0x8ef8fa75000000000000000000000000000000000000000000000007608b0c13426c7dde args=[136083875842229042654 [1.36e20]]] test_PrecisionLoss(uint256) (runs: 0, μ: 0, ~: 0)
Logs:
  Bound result 3424681274

Traces:
  [174] BugTest::setUp()
    └─ ← [Return]

  [7383] BugTest::test_PrecisionLoss(136083875842229042654 [1.36e20])
    ├─ [0] console::log("Bound result", 3424681274 [3.424e9]) [staticcall]
    │   └─ ← [Stop]
    ├─ [0] VM::assertEq(3424377600 [3.424e9], 3424681274 [3.424e9], "Precision Loss") [staticcall]
    │   └─ ← [Revert] Precision Loss: 3424377600 != 3424681274
    └─ ← [Revert] Precision Loss: 3424377600 != 3424681274

Suite result: FAILED. 0 passed; 1 failed; 0 skipped;

Mitigation

To mitigate this issue, consider using a more precise calculation method. One approach is to store the start of the first week as a constant and calculate periods based on that. For example:

uint256 public constant WEEK_START = 1672531200; // Example: Jan 1, 2023 00:00:00 UTC

function calculateWeek(uint256 timestamp) public view returns (uint256) {
    return (timestamp - WEEK_START) / 1 weeks;
}

function calculateWeekStart(uint256 timestamp) public view returns (uint256) {
    return WEEK_START + (calculateWeek(timestamp) * 1 weeks);
}