Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

user can't withdraw some tokens like (USDT)

745fe9f9c2

Summary

The withdraw function allows users to withdraw a specified token amount from their balance, provided certain conditions are met. It operates differently depending on whether the token is a native token (like Ether) or an ERC20 token.

Vulnerability Details

the problem with _safe_transfer_from function , If the token is USDT, the function will revert. USDT does not return a value upon transfer, which can cause the safe transfer method to fail and trigger a revert.

function _safe_transfer_from(
address token,
address from,
address to,
uint256 amount
) internal {
(bool success, ) = token.call(
abi.encodeWithSelector(TRANSFER_FROM_SELECTOR, from, to, amount)
);
​
if (!success) {
revert TransferFailed();
}
}

Impact

user can't withdraw tokens

POC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;
​
// author: mgf15
// run with a mainnet --fork-url such as:
// forge test --fork-url https://rpc.ankr.com/eth
​
import "forge-std/Test.sol";
​
// temporary interface for minting USDT
// should be implemented more extensively, and organized somewhere
interface IUSDT {
function balanceOf(address account) external view returns (uint256);
function transfer(address to, uint256 value) external;
​
}
​
contract USDTTest is Test {
bytes4 private constant APPROVE_SELECTOR =
bytes4(keccak256(bytes("approve(address,uint256)")));
bytes4 private constant TRANSFER_FROM_SELECTOR =
bytes4(keccak256(bytes("transferFrom(address,address,uint256)")));
​
// USDC contract address on mainnet
IUSDT usdt = IUSDT(0xdAC17F958D2ee523a2206206994597C13D831ec7);
​
address owner = makeAddr("owner");
function setUp() public {
vm.prank(usdt.getOwner());
usdt.transfer(owner,1000e6);
​
}
function testapprove() public {
​
vm.prank(owner);
// set up usdt contract address
address token = address(usdt);
// to address
address to = makeAddr("to");
uint256 amount = 100;
//approve to
token.call(
abi.encodeWithSelector(APPROVE_SELECTOR, owner, to, amount)
);
// will fail due to InvalidFEOpcode
_safe_transfer_from(token,owner,to,10);
}
​
function _safe_transfer_from(
address token,
address from,
address to,
uint256 amount
) internal {
(bool success, ) = token.call(
abi.encodeWithSelector(TRANSFER_FROM_SELECTOR, from, to, amount)
);
​
if (!success) {
revert ();
}
}
}
​

Tools Used

Manual Review

Recommendations

use OZ SafeERC20

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-weird-erc-20-return-boolean-Rescuable

I believe the issues and duplicates do not warrant low severity severity as even if the call to transfers returns false instead of reverting, there is no impact as it is arguably correct given there will be insufficient funds to perform a rescue/withdrawal. This will not affect `tillIn()` as there are explicit balance [checks that revert accordingly](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L255-L260) to prevent allowing creation of offers without posting the necessary collateral

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.