First Flight #4: Boss Bridge
First Flight #4
Beginner FriendlyFoundryBridge
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Missing validation in `TokenFactory::deployToken` overwrites existing deployed tokens

wafflemakr

Summary

TokenFactory::deployToken does not validate if a token with the same symbol was already deployed, overwriting existing deployed tokens.

Vulnerability Details

Deployed token address in TokenFactory::deployToken is stored in a mapping, with the symbol as key, which is passed as a function parameter. The vulnerability exists when the owner tries to create another token with the same symbol as a previously deployed token, causing the token address to be overwritten. The contract does not validate if a token with the same symbol already exists before deploying a new one, so calling TokenFactory::getTokenAddressFromSymbol will always return the last token deployed with that symbol.

Proof of Concept

function testAddTokensSameSymbol() public {
vm.prank(owner);
address tokenAddress = tokenFactory.deployToken("TEST", type(L1Token).creationCode);
assertEq(tokenFactory.getTokenAddressFromSymbol("TEST"), tokenAddress);
​
vm.prank(owner);
address tokenAddress2 = tokenFactory.deployToken("TEST", type(L1Token).creationCode);
assertEq(tokenFactory.getTokenAddressFromSymbol("TEST"), tokenAddress2);
assertNotEq(tokenFactory.getTokenAddressFromSymbol("TEST"), tokenAddress);
}

Executing PoC

forge test --mt testAddTokensSameSymbol
[⠒] Compiling...
No files changed, compilation skipped
​
Running 1 test for test/TokenFactoryTest.t.sol:TokenFactoryTest
[PASS] testAddTokensSameSymbol() (gas: 1044044)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.10ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

Low, as the function TokenFactory::getTokenAddressFromSymbol is not really used in crucial parts of the bridge protocol.

Tools Used

  • Manual Review

  • Foundry

Recommendations

Consider storing token address using a uint256 as the key of the mapping and not the string. Its not recommended to use strings as keys anyways. The id can be a uint256 that increases by one every time the token is deployed.

Alternatively, if the owner wants to use the symbol as the key of the mapping, consider adding a validation before deploying the token, checking that the mapping has no address stored with that given token symbol.

function deployToken(string memory symbol, bytes memory contractBytecode) public onlyOwner returns (address addr) {
+ require(s_tokenToAddress[symbol] == address(0), "TokenFactory: token already exists");
assembly {
addr := create(0, add(contractBytecode, 0x20), mload(contractBytecode))
}
s_tokenToAddress[symbol] = addr;
emit TokenDeployed(symbol, addr);
}
Updates

Lead Judging Commences

0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

deployToken: non-unique symbol for tokens

Support

FAQs

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