L2StandardERC20.sol
has a flaw in token's reinitialization logic cause after a while the governor would not be able to update the token's metadata
#120
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
sponsor disputed
Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
unsatisfactory
does not satisfy C4 submission criteria; not eligible for awards
Lines of code
https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/zksync/contracts/bridge/L2StandardERC20.sol#L111-L128
https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/zksync/contracts/bridge/L2StandardERC20.sol#L134-L139
Vulnerability details
Proof of Concept
Take a look at https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/zksync/contracts/bridge/L2StandardERC20.sol#L111-L128
This function is used to reinitialize or update a token's metadata.
Firstly would be key to note that it passes the
version
to theonlyNextVersion()
modifier to ensure that the governor does not accidentally disable future reinitialization of the token, https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/zksync/contracts/bridge/L2StandardERC20.sol#L134-L139Case is with how this whole logic is implemented, would be key to note that the function
_getInitializedVersion()
is inherited from Openzeppelin's implementation, i.e https://github.com/OpenZeppelin/openzeppelin-contracts/blob/0a5fba7a7ed726698477c7772be5d17afe69f118/contracts/proxy/utils/Initializable.sol#L208C1-L227C6Now, evidently this function returns
uint64
value, meaning that protocol instead limits it self to255
versions instead of18,446,744,073,709,551,615
which is over a> 99,9999%
decrease on the amount of generations it should support.Impact
Medium, this is high impact and somewhat low possibility (there needs to be 255 versions ), would be key to note that the current logic for reinitializing the tokens is a core functionality, as it directly syncing of the tokens and symbolizes with the signature logic of the token too, i.e not only is the decoded values for the name and symbol now going to be wrong, there is no possibility of passing in an EIP 712 signature for the token now, since
__ERC20Permit_init(decodedName)
is not going to get called with the correct new name.Recommended Mitigation Steps
Use
version
as is stored from inherited implementations, i.e(uint64)
apply these changes to https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/zksync/contracts/bridge/L2StandardERC20.sol#L111-L139Assessed type
DoS
The text was updated successfully, but these errors were encountered: