sherlock-audit / 2024-02-telcoin-platform-audit-update-judging

3 stars 1 forks source link

Ironsidesec - Tokens that revert on zero amount approval cannot be used as fee token or bridge to Polygon #69

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 8 months ago

Ironsidesec

medium

Tokens that revert on zero amount approval cannot be used as fee token or bridge to Polygon

Summary

There are few tokens that revert if zero approval action is done. And Telcoin's BridgeRelay.transferERCToBridge() attempts to force approve to zero and then increases its allowance to set amount. So this will revert.

Vulnerability Detail

The tokens like (line 94)BNB on mainnet revert if approval call is made with 0 amount. BridgeRelay.transferERCToBridge() & AmirX._buyBack() does force approve to zero before increasing allowance. So, the revert happens and tokens like these cannot used by Telcoin system.

Logs

  [28485] ZeroApprovalTest::testZeroApprovalPOC()
    ├─ [22619] 0xB8c77482e45F1F44dE1745F52C74426C631bDD52::approve(0x0000000000000000000000000000000000000001, 1000000000000000000 [1e18])
    │   └─ ← true
    ├─ [239] 0xB8c77482e45F1F44dE1745F52C74426C631bDD52::approve(0x0000000000000000000000000000000000000001, 0)
    │   └─ ← EvmError: Revert
    ├─ [239] 0xB8c77482e45F1F44dE1745F52C74426C631bDD52::approve(0x0000000000000000000000000000000000000001, 0)
    │   └─ ← EvmError: Revert
    └─ ← revert: SafeERC20: low-level call failed

POC

Install forge by,

  1. forge init --force then run
  2. forge i openzeppelin/openzeppelin-contracts and then run
  3. forge t --mt testZeroApprovalPOC -vvvv --fork-url https://rpc.notadegen.com/eth
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.7;

import "forge-std/Test.sol";
import {SafeERC20} from "../lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
import {IERC20} from "../lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";

contract ZeroApprovalTest is Test {

    function setUp() public {}

    function testZeroApprovalPOC() public {

        IERC20 bnb = IERC20(0xB8c77482e45F1F44dE1745F52C74426C631bDD52);

        bnb.approve(address(1), 1e18);

        SafeERC20.forceApprove(bnb, address(1), 0);
    }
}

Impact

Tokens that revert on zero amount approval cannot be used by Telcoin system (using as fee token or bridging BNB)

Code Snippet

https://github.com/sherlock-audit/2024-02-telcoin-platform-audit-update/blob/21920190e0772afa18e7f856a036fea3ef5b9635/telcoin-contracts/contracts/swap/AmirX.sol#L168

https://github.com/sherlock-audit/2024-02-telcoin-platform-audit-update/blob/21920190e0772afa18e7f856a036fea3ef5b9635/telcoin-contracts/contracts/bridge/BridgeRelay.sol#L69

Tool used

Manual Review

Recommendation

Implement a try catch mechanism and try force approving if revert is caught then increase the allowance in normal way instead of SaefERC20 library's approval actions.

sherlock-admin4 commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

WangAudit commented:

same as 29; low; no funds lost (due to revert of txn) and not breaking contract's core functionality