hats-finance / VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f

LP token lending protocol
MIT License
2 stars 1 forks source link

Some tokens cannot be added as Reward #5

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

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

Github username: @GalloDaSballo Submission hash (on-chain): 0x9edab194e97a358e3a68d899d2dc41b6d09b1522c97918e7c0af1b938aeaf757 Severity: low severity

Description:

Description

Tokens that don't return a bool will fail because the contract is using approve instead of safeApprove

Instances

https://github.com/hats-finance/VMEX-0x41547b88e8d46bfdb6327c0c3ab4b5a1cffb11cd/blob/4bc577756e15418b919a49a1bdec0a98fd39bdbd/packages/contracts/contracts/protocol/incentives/ExternalRewardDistributor.sol#LL61C24-L61C31

    IERC20(underlying).approve(stakingContract, type(uint).max);

Am marking as Low because the admin will be unable to add the token, meaning no tokens will be lost

Also I am aware that VMEX is launching on Optimism so this finding may be downgraded for that reason

Steps to reproduce

  1. Call Approve on USDT, using ERC20 interface, the call will revert, because USDT doesn't return a bool

Expected behavior

Transfer should work

Actual behavior

Tx will revert

Screenshots

If applicable, add screenshots to help explain your problem.

Additional information

See POC of a demo

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.19;

import {Test} from "forge-std/Test.sol";

interface IERC20 {

    function approve(address, uint256) external returns (bool);
}

contract USDTLike {
    uint256 balance;
    function approve(address, uint256 amount) external {
        balance += amount;
        // Return nothing
    }
}
contract UsdtRevertDemo is Test {

    IERC20 usdt;
    function setUp() public {
        usdt = IERC20(address(new USDTLike()));
    }

    function test_usdt() public {
        usdt.approve(address(1), 123);
    }
}

Which will revert

Running 1 test for test/CdpID.invariants.t.sol:UsdtRevertDemo
[FAIL. Reason: EvmError: Revert] test_usdt() (gas: 27672)
Test result: FAILED. 0 passed; 1 failed; finished in 9.55ms

Failing tests:
Encountered 1 failing test in test/CdpID.invariants.t.sol:UsdtRevertDemo
[FAIL. Reason: EvmError: Revert] test_usdt() (gas: 27672)

You can verify that tokens such as USDT do not return a boolean https://etherscan.io/token/0xdac17f958d2ee523a2206206994597c13d831ec7

Mitigation

Use the safeERC20 from OZ

ksyao2002 commented 1 year ago

Looks like on OP, USDT returns a bool: https://optimistic.etherscan.io/token/0x94b008aa00579c1307b0ef2c499ad98a8ce58e58#code.

Are you aware of any other token on OP that may have this issue?

GalloDaSballo commented 1 year ago

I would say for OP there's no such token

ksyao2002 commented 1 year ago

OK thanks. Regardless I have used safeApprove as it is best practice. Thanks for the report.