search

sherlock-audit / 2022-11-nounsdao-judging

4 stars 0 forks source link

Koolex - Non-negligible precision loss for tokens that have small decimals #56

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Koolex

high

Non-negligible precision loss for tokens that have small decimals

Summary

Non-negligible precision loss in ratePerSecond() function for tokens that have less than 6 decimals such as GUSD (Gemini dollar).

Vulnerability Detail

Let's assume:

  1. Stream amount = 1M,
  2. Duration = 31_557_600 seconds (1 year)

We would have

Let's apply the math on 3 tokens USDC, GUSD and WETH

As you can see, the ratePerSecond calculation results in non-negligible loss if the token for example has 2 decimals unlike tokens with 6 or 18 decimals. As per the sponsor, the contract should support popular ERC20 tokens.

Impact

Code Snippet

pragma solidity ^0.8.17;

import "forge-std/Test.sol";
import { ERC20Mock } from "openzeppelin-contracts/mocks/ERC20Mock.sol";
import { StreamFactory } from "../src/StreamFactory.sol";
import { Stream } from "../src/Stream.sol";
import { IStream } from "../src/IStream.sol";

contract StreamTest is Test {
    uint256 constant DURATION = 1000;
    uint256 constant STREAM_AMOUNT = 2000;

    uint256 startTime;
    uint256 stopTime;
    address payer = address(0x11);
    address recipient = address(0x22);

    event TokensWithdrawn(address indexed msgSender, address indexed recipient, uint256 amount);

    event StreamCancelled(
        address indexed msgSender,
        address indexed payer,
        address indexed recipient,
        uint256 payerBalance,
        uint256 recipientBalance
    );

    ERC20Mock token;
    Stream s;
    StreamFactory factory;

    function setUp() public virtual {
        token = new ERC20Mock("Mock Token", "MOCK", address(1), 0);
        factory = new StreamFactory(address(new Stream()));

        startTime = block.timestamp;
        stopTime = block.timestamp + DURATION;
    }
}

contract StreamWithLowDecimals is StreamTest {
    function setUp() public override {
        super.setUp();
    }
     function test_ratePerSecond_USDC() public {
        uint256 streamAmount = 1_000_000 * 1e6; // 1M USDC 
        uint256 duration = 31_557_600; // 365.25 days

        startTime = block.timestamp;
        stopTime = startTime + duration;

        s = Stream(
            factory.createStream(
                payer, recipient, streamAmount, address(token), startTime, stopTime
            )
        );

        // USDC
        // streamAmount/duration = 0.031688087814029
        // after one second
        vm.warp(startTime+1); 
        console.log('USDC Recipient Balance:  %d = 0.031688',s.balanceOf(recipient)); // 31688 = 0.031688 USDC
    }

    function test_ratePerSecond_GUSD() public {
        uint256 streamAmount = 1_000_000 * 1e2; // 1M GUSD 
        uint256 duration = 31_557_600; // 365.25 days

        startTime = block.timestamp;
        stopTime = startTime + duration;

        s = Stream(
            factory.createStream(
                payer, recipient, streamAmount, address(token), startTime, stopTime
            )
        );

        // GUSD
        // streamAmount/duration = 0.031688087814029
        // after one second
        vm.warp(startTime+1); 
        console.log('GUSD Recipient Balance:  %d = 0.03',s.balanceOf(recipient)); // 3 = 0.03 GUSD
    }

    function test_ratePerSecond_WETH() public {
        uint256 streamAmount = 1_000_000 * 1e18; // 1M WETH 
        uint256 duration = 31_557_600; // 365.25 days

        startTime = block.timestamp;
        stopTime = startTime + duration;

        s = Stream(
            factory.createStream(
                payer, recipient, streamAmount, address(token), startTime, stopTime
            )
        );

        // WETH
        // streamAmount/duration = 0.031688087814029
        // after one second
        vm.warp(startTime+1); 
        console.log('WETH Recipient Balance:  %d = 0.031688087814028950',s.balanceOf(recipient)); // 31688087814028950 = 0.031688087814028950 WETH
    }

}

Then run:

forge test --match-path test/StreamDecimals.t.sol -vv

You should get the following logs on the console:

[PASS] test_ratePerSecond_GUSD() (gas: 167284)
Logs:
  GUSD Recipient Balance:  3 = 0.03

[PASS] test_ratePerSecond_USDC() (gas: 167294)
Logs:
  USDC Recipient Balance:  31688 = 0.031688

[PASS] test_ratePerSecond_WETH() (gas: 167339)
Logs:
  WETH Recipient Balance:  31688087814028950 = 0.031688087814028950

Tool used

Manual Review

Recommendation

Consider normalizing the token to 18 decimals upon initializing the stream.

davidbrai commented 1 year ago

The analysis here is incorrect.

For GUSD (2 decimals)

Math: duration = 31,557,600 (1 year) stream amount = 1,000,000 = 1e6 token amount = 100,000,000 = 1e8

ratePerSecond() = 1e6 * 1e8 / 31557600 = 3168808.781402895 which rounds down to 3168808, representing 3.168808 GUSD tokens per seconds, which is $0.03168808 per second The non rounded ratePerSecond is: $1000000/31557600 = $0.03168808781402895 per second

the error per second is 0.03168808781402895 - 0.03168808 = $0.00000000781402895 over a year that is: 0.00000000781402895 * 31557600 = $0.24659199999252

to be convinced with code you can add the following lines at the end of the GUSD test:

        vm.warp(stopTime - 1);
        console.log("GUSD Recipient Balance:  %d", s.balanceOf(recipient));

        vm.warp(stopTime);
        console.log("GUSD Recipient Balance:  %d", s.balanceOf(recipient));

the logs will be:

  GUSD Recipient Balance:  99999972
  GUSD Recipient Balance:  100000000

so at 1 second before the stream ends, the recipient balance is $999,999.72

hrishibhat commented 1 year ago

The precision loss appears to be negligible as shown by the sponsor comment.