Part 2

Summary

The TradingAccount.load function computes storage slots with a bitmask that eliminates the lowest byte, causing different tradingAccountId values to map to the same slot and allowing account data to overlap.

The operation & ~bytes32(uint256(0xff)) zeros out the lowest 8 bits of the computed hash. As a result, two tradingAccountId values that differ only in these lower 8 bits will produce the same storage slot. This behavior violates the guarantee of unique storage isolation for each trading account and permits multiple accounts to share the same storage. An attacker can deliberately choose a tradingAccountId that collides with another account’s ID, thereby bypassing the ownership check in the verifySender function and corrupting account data.

Impact

When multiple trading accounts share the same storage slot, critical fields—such as the owner address—are overwritten, enabling unauthorized access and control. The consequence is that an attacker can exploit the collision to take over another user’s account, manipulate balances, or execute unauthorized transactions, resulting in severe financial loss and systemic compromise.

The vulnerability occurs deterministically under standard Solidity behavior unless the protocol enforces that all tradingAccountId values have their lower 8 bits set to zero. Without such constraints, any tradingAccountId value is free to be chosen, making collisions easy to create. Therefore, this vulnerability will occur unless the masking is removed or tradingAccountId values are strictly controlled.

Tools Used

Recommendations

Remove the masking operation from the storage slot calculation to ensure that each tradingAccountId maps to a unique slot.

This change uses the full 256-bit hash to compute the storage slot, preserving uniqueness and isolating the data of each trading account.

Note: I'm currently marking this vulnerability as Low severity under the assumption the masking operation is an intentional design decision meant to enforce trading account IDs to be multiples of 256. However, if this design is not intentional and the behavior was unintended, the vulnerability must be escalated due to its potential to allow account storage collisions and unauthorized access.