51887 sc medium safeapprove will cause revert of usdt and similar erc20 token

Submitted on Aug 6th 2025 at 13:16:42 UTC by @SAAJ for Attackathon | Plume Network

• Report ID: #51887

• Report Type: Smart Contract

• Report severity: Medium

• Target: https://github.com/immunefi-team/attackathon-plume-network-nucleus-boring-vault/blob/main/src/base/Roles/TellerWithMultiAssetSupportPredicateProxy.sol

• Impacts:

  • Smart contract unable to operate due to lack of token funds

Description

Summary: Checking the boolean return of ERC20 approve breaks protocol compatibility with USDT and similar tokens because they do not return true on success. Repeated calls to safeApprove will revert for such tokens unless allowance is explicitly first set to zero.

Vulnerability Details

The TellerWithMultiAssetSupportPredicateProxy contract calls a solmate-based safeApprove in its deposit and depositAndBridge methods:

depositAsset.safeApprove(address(vault), depositAmount);

depositAsset is the ERC20 token that will be approved to the vault.

USDT (and similar non-EIP-20-compliant tokens) enforce that changing a non-zero allowance to another non-zero allowance requires first setting it to zero. The USDT code causing this behavior:

// To change the approve amount you first have to reduce the addresses`
//  allowance to zero by calling `approve(_spender, 0)` if it is not
//  already 0 to mitigate the race condition described here:
//  https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
require(!((_value != 0) && (allowed[msg.sender][_spender] != 0)));

Because the contract calls safeApprove without ensuring the existing allowance is zero, a subsequent safeApprove for USDT (or similar tokens) will revert. This affects repeated calls to deposit and depositAndBridge by the same user/token combination, causing those calls to fail.

Affected locations (code references)

• https://github.com/immunefi-team/attackathon-plume-network-nucleus-boring-vault/blob/0ee676b5715075c26db6706960fd49ab59b587fc/src/base/Roles/TellerWithMultiAssetSupportPredicateProxy.sol#L89

• https://github.com/immunefi-team/attackathon-plume-network-nucleus-boring-vault/blob/0ee676b5715075c26db6706960fd49ab59b587fc/src/base/Roles/TellerWithMultiAssetSupportPredicateProxy.sol#L146

Recommendation

Properly handle approvals by checking whether an existing non-zero allowance exists; if so, first set it to zero before setting it to the new non-zero allowance. Apply this check to both deposit and depositAndBridge.

Suggested patch (illustrative):

        ERC20 vault = ERC20(teller.vault());

+       // If existing approval is non-zero -> set it to zero
+       if (depositAsset.allowance(msg.sender, address(vault)) != 0) {
+           depositAsset.safeApprove(address(vault), uint8(0));
+       }

        //approve vault to take assets from proxy
        depositAsset.safeApprove(address(vault), depositAmount);

Apply equivalent logic to both deposit and depositAndBridge methods so repeated deposits with USDT-like tokens don't revert.

Proof of Concept

The following test reproduces the revert when calling safeApprove for USDT more than once. The snippet demonstrates a first successful approve and a second approve that reverts:

// forge t -f $fork_rpc_url --etherscan-api-key $key_Api --mt test_USDTApprove -vvv
function test_USDTApprove() external {
    ERC20 USDT = ERC20(0xdAC17F958D2ee523a2206206994597C13D831ec7); // usdt address
    address random_User = 0x46340b20830761efd32832A74d7169B29FEB9758; // random User having USDT
    uint256 amount_Approve = 1000 * 1e6;

    vm.startPrank(random_User); // mock call made by user
    USDT.safeApprove(address(this), amount_Approve); // approving amount 
    uint256 approved_Balance = (vault).allowance(random_User, address(vault)); // caching approved amount
    console.log("Approved Balance: %e", approved_Balance);
    vm.stopPrank();

    vm.startPrank(random_User); // again calling the function when approve exist
    USDT.safeApprove(address(vault), amount_Approve); // this line reverts
}

Failure log (expanded)

Ran 1 test for test/PlumeTest.t.sol:implementUpgradeTest [FAIL: APPROVE_FAILED] test_USDTApprove() (gas: 41079) Logs: Approved Balance: 1e9

Traces: [41079] implementUpgradeTest::test_USDTApprove() ├─ [0] VM::startPrank(0x46340b20830761efd32832A74d7169B29FEB9758) │ └─ ← [Return] ├─ [26953] TetherToken::approve(implementUpgradeTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 1000000000 [1e9]) │ ├─ emit Approval(owner: 0x46340b20830761efd32832A74d7169B29FEB9758, spender: implementUpgradeTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], v alue: 1000000000 [1e9]) │ └─ ← [Stop] ├─ [1356] TetherToken::allowance(0x46340b20830761efd32832A74d7169B29FEB9758, implementUpgradeTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [stat iccall] │ └─ ← [Return] 1000000000 [1e9] ├─ [0] console::log("Approved Balance: %e", 1000000000 [1e9]) [staticcall] │ └─ ← [Stop] ├─ [0] VM::stopPrank() │ └─ ← [Return] ├─ [0] VM::startPrank(0x46340b20830761efd32832A74d7169B29FEB9758) │ └─ ← [Return] ├─ [903] TetherToken::approve(implementUpgradeTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 1000000000 [1e9]) │ └─ ← [Revert] EvmError: Revert └─ ← [Revert] APPROVE_FAILED

Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 6.49s (2.05s CPU time)

Ran 1 test suite in 7.62s (6.49s CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests: Encountered 1 failing test in test/PlumeTest.t.sol:implementUpgradeTest [FAIL: APPROVE_FAILED] test_USDTApprove() (gas: 41079)

Encountered a total of 1 failing tests, 0 tests succeeded