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 about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
4rdiii Submitter
about 1 year ago
4rdiii Submitter
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year 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.