This repository has been archived by the owner on May 26, 2023. It is now read-only.
0x1337 - Minor: Incorrect and inconsistent storage gap #117
Labels
Duplicate
A valid issue that is a duplicate of an issue with `Has Duplicates` label
Reward
A payout will be made for this issue
Specification
An issue related to the specification (low severity)
0x1337
low
Minor: Incorrect and inconsistent storage gap
Summary
The
ERC721Bridge
base contract is used by both theL1ERC721Bridge
contract and theL2ERC721Bridge
contract, and contains storage gap, including the spec of total storage slot of 50 in code comments. However, the actual storage gap in the contract is not 50.The
CrossDomainMessenger
base contract is used by both theL1CrossDomainMessenger
andL2CrossDomainMessenger
contracts. The contract has comment saying gap size of 41 and the intended total storage slot of 50. The actual total storage gap in this contract is 42 + 6 = 48.It appears that the developer has failed to recognize that
immutable
andconstant
variables do not take any storage slot, in arriving at the incorrect storage slot.Vulnerability Detail
In the contract
ERC721Bridge
, line 25 reserves 49 slots when the comment above says total of 50. BothMESSENGER
andOTHER_BRIDGE
areimmutable
and do not take any storage slot.In the contract
CrossDomainMessenger
, line 137 includes gap of 42, and the number of non constant and non immutable variables declared in this contract is 6, bringing total storage gap to 48.Impact
49 storage slots are reserved instead of the intended spec of 50 slots in
ERC721Bridge
, and 48 storage slots are reserved instead of the intended spec of 50 slots inCrossDomainMessenger
.Code Snippet
https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/universal/ERC721Bridge.sol#L25
https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol#L94-L137
Tool used
Manual Review
Recommendation
Modify the length of the
__gap
array to bring total storage gap to 50 in these contracts.Duplicate of #146
The text was updated successfully, but these errors were encountered: