Open sherlock-admin2 opened 5 months ago
I dont think a try catch will work because if the token doesnt implement that fn at all , it will still revert. hm.
The protocol team fixed this issue in the following PRs/commits: https://github.com/teller-protocol/teller-protocol-v2-audit-2024/pull/36
The Lead Senior Watson signed off on the fix.
pkqs90
medium
LenderCommitmentGroup_Smart.sol
cannot deploy pools with non-string symbol() ERC20s.Summary
In
LenderCommitmentGroup_Smart.sol
, when initializing, it needs to fetch thesymbol()
for the principalToken and collateralToken. However, it assumes thesymbol()
returns a string, which is actually OPTIONAL for ERC20 standards. This means it would fail to create pools for these ERC20s.Vulnerability Detail
First, let's quote the EIP20 to show
symbol()
is optional:The most famous token that uses bytes32 instead of string as
symbol()
return value is MKR.The contest README states that any tokens compatible with Uniswap V3 should be supported, which includes MKR: https://info.uniswap.org/#/tokens/0x9f8f72aa9304c8b593d555f12ef6589cc3a579a2
At last, let's see the code.
_generateTokenNameAndSymbol
assumes that both principalToken and collateralToken has asymbol()
function that returnsstring
, which is inaccurate. This would revert if we try to create a pool with MKR.Impact
LenderCommitmentGroup_Smart
pools cannot be created for ERC20s that does not implementfunction symbol() public view returns (string)
.Code Snippet
Tool used
Manual review
Recommendation
Consider using a try-catch, an example would be: