Submission Details
Severity:
Fee-on-transfer tokens will always revert on creating Escrow contract
Summary
EscrowFactory will not deploy a new Escrow contract using Fee-on-transfer tokens as payment
Vulnerability Details
The only parameter for amounts to fund the Escrow contract is price
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/EscrowFactory.sol#L20-L27
function newEscrow(
uint256 price,
IERC20 tokenContract,
address seller,
address arbiter,
uint256 arbiterFee,
bytes32 salt
) external returns (IEscrow) {
even if this is increased to account for the transfer fee, the code still requires the balance of the new Escrow contract to be the same as price, which is not possible as fees will always be deducted from the price value.
PoC
This POC can be run as a forge test.
Contract for Fee-on-transfer token (copy/paste to file FeeERC20.sol)
// SPDX-License-Identifier: UNLICENSED
// slither-disable-next-line solc-version
import "../lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
/// @dev Lottery token contract. The token has a fixed initial supply.
/// Additional tokens can be minted after each draw is finalized. Inflation rates (per draw) are defined for each year.
contract FeeERC20 is ERC20 {
uint256 public constant INITIAL_SUPPLY = 1_000_000_000e18;
address _owner;
uint256 public fees;
constructor(uint256 _fees) ERC20("FeeToken", "FT") {
_owner = msg.sender;
fees = _fees;
}
//modifier
modifier onlyOwner() {
require(msg.sender==_owner,"Notthe Owner!");
_;
}
function mint(address account, uint256 amount) external onlyOwner {
_mint(account, amount);
}
function transfer(address to,uint256 amount) public override returns (bool){
uint256 newAmount = amount - fees;
return super.transfer(to,newAmount);
}
function transferFrom(address src,address to,uint256 amount) public override returns (bool){
uint256 newAmount = amount - fees;
return super.transferFrom(src,to,newAmount);
}
}
Testing contract for foundry, the code expects a revert with IEscrow.Escrow__MustDeployWithTokenBalance
(copy/paste to file FeeTokenTest.t.sol)
// SPDX-License-Identifier: MIT
import "forge-std/Test.sol";
import {IEscrow, Escrow} from "../src/Escrow.sol";
import {EscrowFactory} from "../src/EscrowFactory.sol";
import "../src/FeeERC20.sol";
contract FeeTokenTest is Test {
bytes32 public constant SALT1 = bytes32(uint256(keccak256(abi.encodePacked("test"))));
EscrowFactory public escrowFactory;
address public DEPLOYER = makeAddr("DEPLOYER");
address public BUYER = makeAddr("BUYER");
address public SELLER = makeAddr("SELLER");
address public ARBITER = makeAddr("ARBITER");
uint256 public constant ARBITER_FEE = 1e16;
IEscrow public escrow;
uint256 public buyerAward = 0;
FeeERC20 testToken;
function setUp() external {
vm.startPrank(DEPLOYER);
// Create Fee-On-transfer ERC20 token
testToken = new FeeERC20(0.001 ether);
// Mint tokens to parties
testToken.mint(BUYER,100 ether);
testToken.mint(SELLER,10 ether);
// Create new escrowFactory
escrowFactory = new EscrowFactory();
vm.stopPrank();
}
function testHappyPath() public {
vm.startPrank(BUYER);
// approve tokens to EscrowFactory
testToken.approve(address(escrowFactory), 10 ether);
// Expect revert IEscrow.Escrow__MustDeployWithTokenBalance.selector
vm.expectRevert(IEscrow.Escrow__MustDeployWithTokenBalance.selector);
escrow = escrowFactory.newEscrow(10 ether, IERC20(testToken), SELLER, ARBITER, ARBITER_FEE, SALT1);
vm.stopPrank();
}
}
Impact
Excludes the usage of Fee-on-transfer tokens from being used as payment.
Tools Used
Manual Audit
Foundry
Recommendations
You could consider implementing a separate create function in the EscrowFactory as below:
(Full code including IEscrowFactory and forge test included below)
function newFOTEscrow(
uint256 contractFundAmount,
uint256 price,
IERC20 tokenContract,
address seller,
address arbiter,
uint256 arbiterFee,
bytes32 salt
) external returns (IEscrow) {
address computedAddress = computeEscrowAddress(
type(Escrow).creationCode,
address(this),
uint256(salt),
price,
tokenContract,
msg.sender,
seller,
arbiter,
arbiterFee
);
tokenContract.safeTransferFrom(msg.sender, computedAddress, contractFundAmount);
Escrow escrow = new Escrow{salt: salt}(
price,
tokenContract,
msg.sender,
seller,
arbiter,
arbiterFee
);
if (address(escrow) != computedAddress) {
revert EscrowFactory__AddressesDiffer();
}
emit EscrowCreated(address(escrow), msg.sender, seller, arbiter);
return escrow;
}
IEscrowFactory
/// @notice deploy a new escrow contract. The escrow will hold all the funds. The buyer is whoever calls this function.
/// @param contractFundAmount the amount of tokens to fund the new Escrow contract including the Fee-on-Transfer.
/// @param price the price of the escrow. This is the agreed upon price for this service.
/// @param tokenContract the address of the token contract to use for this escrow, ie USDC, WETH, DAI, etc.
/// @param seller the address of the seller. This is the one receiving the tokens.
/// @param arbiter the address of the arbiter. This is the one who will resolve disputes.
/// @param arbiterFee the fee the arbiter will receive for resolving disputes.
/// @param salt the salt to use for the escrow contract. This is used to prevent replay attacks.
/// @return the address of the newly deployed escrow contract.
function newFOTEscrow(
uint256 contractFundAmount,
uint256 price,
IERC20 tokenContract,
address seller,
address arbiter,
uint256 arbiterFee,
bytes32 salt
) external returns (IEscrow);
Foundry test
// SPDX-License-Identifier: MIT
import "forge-std/Test.sol";
import {IEscrow, Escrow} from "../src/Escrow.sol";
import {EscrowFactory} from "../src/EscrowFactory.sol";
import "../src/FeeERC20.sol";
contract FeeTokenTest is Test {
bytes32 public constant SALT1 = bytes32(uint256(keccak256(abi.encodePacked("test"))));
EscrowFactory public escrowFactory;
address public DEPLOYER = makeAddr("DEPLOYER");
address public BUYER = makeAddr("BUYER");
address public SELLER = makeAddr("SELLER");
address public ARBITER = makeAddr("ARBITER");
uint256 public constant ARBITER_FEE = 1e16;
IEscrow public escrow;
uint256 public buyerAward = 0;
FeeERC20 testToken;
function setUp() external {
vm.startPrank(DEPLOYER);
// Create Fee-On-transfer ERC20 token
testToken = new FeeERC20(0.001 ether);
// Mint tokens to parties
testToken.mint(BUYER,100 ether);
testToken.mint(SELLER,10 ether);
// Create new escrowFactory
escrowFactory = new EscrowFactory();
vm.stopPrank();
}
function testFOTPath() public {
vm.startPrank(BUYER);
// approve tokens to EscrowFactory
testToken.approve(address(escrowFactory), 12 ether);
// Just sending too many tokens to show the concept
escrow = escrowFactory.newFOTEscrow(12 ether,10 ether, IERC20(testToken), SELLER, ARBITER, ARBITER_FEE, SALT1);
vm.stopPrank();
assertEq(escrow.getPrice(), 10 ether);
assertEq(address(escrow.getTokenContract()), address(testToken));
assertEq(escrow.getBuyer(), BUYER);
assertEq(escrow.getSeller(), SELLER);
assertEq(escrow.getArbiter(), ARBITER);
assertEq(escrow.getArbiterFee(), ARBITER_FEE);
}
}
Prize pool breakdown
Total prize
40,000 USDC
nSLOC
186215 USDC / LOC
37,000 USDC
3,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.