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

The `PoolFactory` contract wont work with tokens like MKR which have bytes32 `name` and `symbol`

4rdiii

[L-01] The PoolFactory contract wont work with tokens like MKR which have bytes32 name and symbol

Description:
Some ERC20 tokens like MKR (Maker governance token) have their name and symbol methods return bytes32 instead of strings.
Impact:
As stated in onboarding questions, the poolFactory should work with almost every ERC20, but using some tokens like Maker would result in a revert.
Proof of Concept:
Since the default OpenZeppelin IERC20 doesn't support these tokens, it will revert. The test below, used with the --fork-url flag, tests the factory in a Mainnet forked environment.

forge test --mt testCreatePoolDoesntWorkWithBytes32 --fork-url $MAINNET_RPC_URL
function testCreatePoolDoesntWorkWithBytes32() public {
address makerToken = 0x9f8F72aA9304c8B593d555F12eF6589cC3A579A2;
address usdcToken = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48;
// dosent work with bytes32 tokens
vm.expectRevert();
address poolAddressMKR = factory.createPool(makerToken);
// works fine with other tokens
address poolAddressUSDC = factory.createPool(address(usdcToken));
​
assertEq(poolAddressUSDC, factory.getPool(address(usdcToken)));
assertEq(address(usdcToken), factory.getToken(poolAddressUSDC));
}

Recommended Mitigation:
One easy way to fix this is to set the name and symbol as createPool function input parameters and don't get them from the ERC20 metadata.

- function createPool(address tokenAddress) external returns (address) {
+ function createPool(address tokenAddress, string memory _name, string memory _symbol) external returns (address) {
if (s_pools[tokenAddress] != address(0)) {
revert PoolFactory__PoolAlreadyExists(tokenAddress);
}
string memory liquidityTokenName = string.concat(
"T-Swap ",
- IERC20(tokenAddress).name()
+ _name
);
string memory liquidityTokenSymbol = string.concat(
"ts",
- IERC20(tokenAddress).name()
+ _symbol
);
​
TSwapPool tPool = new TSwapPool(
tokenAddress,
i_wethToken,
liquidityTokenName,
liquidityTokenSymbol
);
s_pools[tokenAddress] = address(tPool);
s_tokens[address(tPool)] = tokenAddress;
emit PoolCreated(tokenAddress, address(tPool));
return address(tPool);
}
Updates

Appeal created

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
4rdiii Submitter
12 months ago
4rdiii Submitter
12 months ago
inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

The `PoolFactory` contract wont work with tokens like MKR which have bytes32 `name` and `symbol`

Support

FAQs

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