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
approvebreaks protocol compatibility with USDT and similar tokens because they do not returntrueon success. Repeated calls tosafeApprovewill 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