First Flight #18: T-Swap
First Flight #18
Beginner FriendlyDeFiFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Hardcoded decimal value leads to incorrect conversion when ERC20 does not use 18 decimals.

poolig4n

Hardcoded decimal value leads to incorrect conversion when ERC20 does not use 18 decimals.

Description
The convenience function below calls getOutputAmountBasedOnInput() with a hardcoded value of 1e18 for the parameter inputAmount. This assumes the ERC token will use 18 decimals which is not true for all ERC tokens. eg. USDC uses 6 decimals and WBTC uses 8.

  • getPriceOfOnePoolTokenInWeth()

​
function getPriceOfOnePoolTokenInWeth() external view returns (uint256) {
return
getOutputAmountBasedOnInput(
1e18, //@audit what if it's a USDC with 6 decimals // input amount - 1 pool token
i_poolToken.balanceOf(address(this)), // input reserves
i_wethToken.balanceOf(address(this))
);
}
​

This leads to an incorrect calculation of the value of one token in the pool.

The documentation states "Any ERC20 token":

Audit Scope Details

  • Solc Version: 0.8.20

  • Chain(s) to deploy contract to: Ethereum

  • Tokens:
    - Any ERC20 token

Impact

The protocol does not correctly support all ERC tokens even though the documentation states all ERC tokens are supported. When tokens that utilise a decimal value other than 18 are used in the pool the conversion calculations above will be incorrect.

Severity
This function would likely be used to make decisions based on expected pricing and therefore could pose a significant risk to end-users.

Medium Impact
High Likelihood
Severity: Medium

Proof of Concept

The following standalone unit test demonstrates the issue:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import { Test, console } from "forge-std/Test.sol";
import { PoolFactory } from "../../src/PoolFactory.sol";
import { TSwapPool } from "../../src/TSwapPool.sol";
​
import { ERC20Mock } from "@openzeppelin/contracts/mocks/token/ERC20Mock.sol";
import { IERC20 } from "@openzeppelin/contracts/interfaces/IERC20.sol";
import {USDCMock} from "./USDCMock.sol"; //usdc
import {WETHMock} from "./WETHMock.sol"; //weth
​
contract ExperimentTest is Test {
​
TSwapPool pool;
USDCMock poolToken;
WETHMock weth;
PoolFactory poolFactory;
​
address liquidityProvider = makeAddr("liquidityProvider");
address user = makeAddr("user");
​
address poolAddress;
​
function setUp() public {
poolToken = new USDCMock();
weth = new WETHMock();
poolFactory = new PoolFactory( address(weth) );
poolAddress = poolFactory.createPool( address(poolToken) );
pool = TSwapPool(poolFactory.getPool( address(poolToken) ));
}
​
function testDecimals() public {
uint256 oneToken = 1e6;
uint256 liquidity = oneToken * 1000;
// generate some liquidity
weth.mint(liquidityProvider, liquidity);
poolToken.mint(liquidityProvider, liquidity);
​
// allocate some weth to the user to use for swapping
weth.mint(user, oneToken);
//poolToken.mint(user, oneToken);
​
// add liquidity to the pool
vm.startPrank(liquidityProvider);
weth.approve(address(pool), liquidity);
poolToken.approve(address(pool), liquidity);
​
//deposit liquidity into the pool at 1:1 ratio
pool.deposit(liquidity, liquidity, liquidity, uint64(block.timestamp));
vm.stopPrank();
​
//now we have a 1:1 ratio of Weth:PoolToken (which is 1e6)
// call the function to get the price
uint256 priceInWethForOnePoolToken = pool.getPriceOfOnePoolTokenInWeth();
​
// Enable this function to demonstrate the difference between 1e18 and 1e6
// You'll also need to add getPriceOfOneUSDCPoolTokenInWeth to TWSwapPool.sol
//uint256 priceInWethForOneUSDCPoolToken = pool.getPriceOfOneUSDCPoolTokenInWeth();
uint256 oneWeth = 1e18;
​
console.log("One Pool Token: ", oneToken);
console.log("One Weth Token: ", oneWeth);
​
console.log("priceInWethForOnePoolToken: ",priceInWethForOnePoolToken);
//Enable this function to demonstrate the difference between 1e18 and 1e6
//console.log("priceInWethForOneUSDCPoolToken: ", priceInWethForOneUSDCPoolToken);
​
}
}
​
​
// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
contract USDCMock is ERC20 {
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
​
constructor() ERC20("USDCMock", "USDCM") { }
​
function decimals() public view virtual override returns (uint8) {
return 18;
}
​
}
​
// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
contract WETHMock is ERC20 {
constructor() ERC20("WETHMock", "WETHM") {}
​
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
}
​

Result with current code:

​
Ran 1 test for test/unit/ExperimentTest.t.sol:ExperimentTest
[PASS] testDecimals() (gas: 239736)
Logs:
One Pool Token: 1000000
One Weth Token: 1000000000000000000
priceInWethForOnePoolToken: 999999998
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.07ms (1.62ms CPU time)
​

Result with implemenation using 1e6 is shown below:

​
// You'd need to add getPriceOfOneUSDCPoolTokenInWeth to TWSwapPool.sol
// function uses 1e6 instead of 1e18
function getPriceOfOneUSDCPoolTokenInWeth() external view returns (uint256) {
return
getOutputAmountBasedOnInput(
1e6, //@audit-experiment USDC with 6 decimals
i_poolToken.balanceOf(address(this)),
i_wethToken.balanceOf(address(this))
);
}
​
​
Ran 1 test for test/unit/ExperimentTest.t.sol:ExperimentTest
[PASS] testDecimals() (gas: 242930)
Logs:
One Pool Token: 1000000
One Weth Token: 1000000000000000000
priceInWethForOnePoolToken: 999999998
priceInWethForOneUSDCPoolToken: 996006
​

Recommended mitigation

  • Create a whitelist of supported ERC20 tokens.

  • Create a mapping from token symbol to decimals and use that as a lookup for calculations rather than expecting 18 decimals for all tokens.

References

https://github.com/Cyfrin/2024-06-t-swap/blob/d1783a0ae66f4f43f47cb045e51eca822cd059be/src/TSwapPool.sol#L452

https://github.com/Cyfrin/2024-06-t-swap/blob/d1783a0ae66f4f43f47cb045e51eca822cd059be/src/TSwapPool.sol#L461

Tools Used

  • Manual review

  • Foundry

Updates

Appeal created

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Hardcoded decimal value leads to incorrect conversion when ERC20 does not use 18 decimals.

Support

FAQs

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