hats-finance / Thorn-protocol-0x1286ecdac50215a366458a14968fbca4bd95067d

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

tokens without decimals won't work #13

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

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

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

Description: Description\ Thorn protocol should be similar to Uniswap and thus should support all sorts of ERC20 tokens, but the problem is that Decimals are optional for the ERC and there can be tokens that don’t implement it. - https://eips.ethereum.org/EIPS/eip-20#decimals

Attack Scenario\ As a result, not all the ERC20 tokens will be supported and those that don’t have decimals will fail in the StableSwapPool::initialize:

function initialize(
      address[N_COINS] memory _coins,
      uint256 _A,
      uint256 _fee,
      uint256 _admin_fee,
      address _owner,
      address _LP
  ) external {
      require(!isInitialized, "Operations: Already initialized");
      require(msg.sender == STABLESWAP_FACTORY, "Operations: Not factory");
      require(_A <= MAX_A, "_A exceeds maximum");
      require(_fee <= MAX_FEE, "_fee exceeds maximum");
      require(_admin_fee <= MAX_ADMIN_FEE, "_admin_fee exceeds maximum");
      isInitialized = true;
      for (uint256 i = 0; i < N_COINS; i++) {
          require(_coins[i] != address(0), "ZERO Address");
          uint256 coinDecimal;
          if (_coins[i] == ROSE_ADDRESS) {
              coinDecimal = 18;
              support_ROSE = true;
          } else {
              coinDecimal = IERC20Metadata(_coins[i]).decimals();
          }

Here the execution tries to retrieve the decimals with a high-level call which will revert in case there is no such function in the ABI of the token contract.

Attachments

  1. Proof of Concept (PoC) File
  2. Revised Code File (Optional)

In order to handle all the valid ERC20s decimals should be queried in a try/catch blocks and in case there is no decimals function is presented the catch statement should assign default of 18 decimals.

omega-audits commented 1 month ago

The ERC20 standard is very generic, and there is, afaik, no claim that all ERC20 tokens are supported (see also #12).