hats-finance / Thorn-protocol-0x1286ecdac50215a366458a14968fbca4bd95067d

GNU General Public License v3.0
0 stars 0 forks source link

`calc_token_amount` reverts when total supply is 0 #65

Open hats-bug-reporter[bot] opened 1 day ago

hats-bug-reporter[bot] commented 1 day ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xd15db2859012b3353cdbabfe2b3a4cca69ef7907050c6234b593ece898633a9c Severity: low

Description: Description\

calc_token_amount doesn't check that total supply is greater than 0. Thus, when calculating the D0 with get_D_mem the D0 becomes 0 and at the return statement of the calc_token_amount division by 0 occurs.

function calc_token_amount(uint256[N_COINS] memory amounts, bool deposit) external view returns (uint256) {
        /**
        Simplified method to calculate addition or reduction in token supply at
        deposit or withdrawal without taking fees into account (but looking at
        slippage).
        Needed to prevent front-running, not for precise calculations!
        */
        uint256[N_COINS] memory _balances = balances;
        uint256 amp = get_A();
        uint256 D0 = get_D_mem(_balances, amp);
        for (uint256 i = 0; i < N_COINS; i++) {
            if (deposit) {
                _balances[i] += amounts[i];
            } else {
                _balances[i] -= amounts[i];
            }
        }
        uint256 D1 = get_D_mem(_balances, amp);
        uint256 token_amount = token.totalSupply();
        uint256 difference;
        if (deposit) {
            difference = D1 - D0;
        } else {
            difference = D0 - D1;
        }
        return (difference * token_amount) / D0;
    }

Attachments

  1. Proof of Concept (PoC) File

Run the following test:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;

import {Test, console} from "forge-std/Test.sol";
import {StableSwapLP} from "../contracts/stableSwap/StableSwapLP.sol";
import {StableSwapFactory} from "../contracts/stableSwap/StableSwapFactory.sol";
import {StableSwapLPFactory} from "../contracts/stableSwap/StableSwapLPFactory.sol";
import {StableSwapTwoPool} from "../contracts/stableSwap/plain-pools/StableSwapTwoPool.sol";
import {StableSwapThreePool} from "../contracts/stableSwap/plain-pools/StableSwapThreePool.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";

contract MockERC20 is ERC20 {
    uint8 public _decimals;

    constructor(string memory name, string memory symbol, uint8 newDecimals) ERC20(name, symbol) {
        _decimals = newDecimals;
    }

    function decimals() public view override returns (uint8) {
        return _decimals;
    }
}

contract POC is Test {
    StableSwapLP public lp;
    address public tokenA;
    address public tokenB;
    StableSwapTwoPool public pool;
    address public owner = makeAddr("owner");
    uint256 public A = 1000; // from the tests
    uint256 public fee = 4_000_000; // from the tests
    uint256 public admin_fee = 5_000_000_000; // from the tests
    address public actor = makeAddr("actor"); // malicious actor
    address public user = makeAddr("user"); // normal user
    uint256 public decimals = 18;

    function setUp() public {
        lp = new StableSwapLP();

        tokenA = address(new MockERC20("TOKENA", "TOKENA", 18));
        tokenB = address(new MockERC20("TOKENB", "TOKENB", 18));

        pool = new StableSwapTwoPool();
        pool.initialize([tokenA, tokenB], A, fee, admin_fee, owner, address(lp));

        lp.setMinter(address(pool));

        vm.stopPrank();
    }

    function test_POCRevert() public {
        uint256 userInitLiq = 100 ether;
        deal(tokenA, user, userInitLiq);
        deal(tokenB, user, userInitLiq);

        vm.startPrank(user);
        vm.expectRevert();
        uint256 lpToGet = pool.calc_token_amount([userInitLiq, userInitLiq], true);
        console.log("LP TO GET :", lpToGet);
    }
}
  1. Revised Code File

Check that the token supply is greater than zero. Else, calculate the return value similar to how add_liquidity works. The following should be added:

if (token_supply > 0) {
            D0 = get_D_mem(old_balances, amp);
}
Giannis443 commented 1 day ago

Adding to the submission, this will result in protocols that integrate with Thorn to revert, when token supply is 0.

cpp-phoenix commented 1 day ago

Duplicate of #64

Giannis443 commented 1 day ago

This submission does not "duplicate" #64. That submission fails to follow the guidelines given by Hats of what a valid submission is. No impact has been given, no root cause, not even a recommendation. Apart from the fact that those are missing, all those information are written first time here, since they are not present in #64 even at the time of writing this reply.

cpp-phoenix commented 1 day ago

Let's leave it to @omega-audits to decide

cpp-phoenix commented 1 day ago

@Giannis443 Your recommended changes doesn't resolve the issue. As the last line in calc_token_amount() will still lead to revert as D0 will be 0 if LP supply is 0.

return (difference * token_amount) / D0;