DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Valid

Enumerated Storage Locations are Not ERC7201 Compatible

[M-01] Enumerated Storage Locations are Not ERC7201 Compatible

Vulnerability Details

Some storage locations that are enumerated by variables such as user address (tradingAccountId) or marketId do not follow ERC7201 standards. For example, while Position::POSITION_LOCATION is defined correctly according to ERC7201, when using the Position::load function, the slot location derived using variables like tradingAccountId and marketId loses ERC7201 compatibility.

library Position {
/// @notice ERC7201 storage location.
@> bytes32 internal constant POSITION_LOCATION =
@> keccak256(abi.encode(uint256(keccak256("fi.zaros.perpetuals.Position")) - 1)) & ~bytes32(uint256(0xff));
.
.
.
function load(uint128 tradingAccountId, uint128 marketId) internal pure returns (Data storage position) {
@> bytes32 slot = keccak256(abi.encode(POSITION_LOCATION, tradingAccountId, marketId));
assembly {
position.slot := slot
}
}
}

Impact

Performing an AND NOT operation with ~bytes32(uint256(0xff)) ensures that the storage slot's rightmost byte is 00. This prepares for a future upgrade when Ethereum switches its storage data structure to Verkle Trees, allowing 256 adjacent slots to be warmed at once. However, the current version ignores this, and slots created this way do not end with 00 as the rightmost byte.
For example, if marketId = 1 and tradingAccountId = 1324, the slot would be:

➜ bytes32 internal constant POSITION_LOCATION =
keccak256(abi.encode(uint256(keccak256("fi.zaros.perpetuals.Position")) - 1)) & ~bytes32(uint256(0xff));
➜ uint128 tradingAccountId = 1324
➜ uint128 marketId = 1
➜ bytes32 slot = keccak256(abi.encode(POSITION_LOCATION, tradingAccountId, marketId));
➜ slot
Type: bytes32
└ Data: 0xf22d8f9b1165be2f6c9bc0d54126c3ba2f5d9c90d6fd5d960e561aa8ddbd57db
➜ POSITION_LOCATION
Type: bytes32
└ Data: 0x130e63f8df15deca2afebe5c8429cf1fe6f7dc4ea8275a46d22b4230a9159200

It is clearly visible that the slot's rightmost byte is not 00 using this formula, thus it is not ERC7201 compatible.

Tools Used

Manual Review

Recommendations

To fix this, the ERC7201 suggested formula can be used to ensure ERC7201 compatibility:

function load(uint128 tradingAccountId, uint128 marketId) internal pure returns (Data storage position) {
- bytes32 slot = keccak256(abi.encode(POSITION_LOCATION, tradingAccountId, marketId));
+ bytes32 slot = keccak256(abi.encode(POSITION_LOCATION, tradingAccountId, marketId)) & ~bytes32(uint256(0xff));
assembly {
position.slot := slot
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Storage computation formula of ERC7201 is not followed. ERC7201 non compliance.

Support

FAQs

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

Give us feedback!