DeFiHardhat
12,000 USDC
View results
Submission Details
Severity: low
Invalid

Risk of Storage Slot Overwrites from Manual Slot Management

Summary

The LibLastReserveBytes library manually manages storage slots, which could lead to overwriting data if not handled with extreme care.

Vulnerability Details

The vulnerability in question arises from the manual management of storage slots within the LibLastReserveBytes library. Solidity provides a built-in storage layout that automatically handles the location and management of state variables. However, when using low-level assembly language to directly interact with storage, developers take on the responsibility of managing the storage layout manually. This increases the risk of storage collisions, where one piece of data can unintentionally overwrite another.

In the storeLastReserves function, the library calculates storage slots based on an index offset from a given slot. Here's the relevant code snippet:

if (n > 2) {
uint256 maxI = n / 2; // number of fully-packed slots
uint256 iByte; // byte offset of the current reserve
for (uint256 i = 1; i < maxI; ++i) {
iByte = i * 64;
assembly {
sstore(
add(slot, i),
add(mload(add(reserves, add(iByte, 32))), shr(128, mload(add(reserves, add(iByte, 64)))))
)
}
}
// ... additional code handling odd number of reserves
}

The vulnerability stems from the use of the add(slot, i) operation within the assembly block. This line of code assumes that the next available storage slot is at position slot + i. However, if there's a miscalculation in the offset i or if the initial slot slot is not correctly determined to be free of existing data, the store operation can overwrite data that is already stored in the calculated slot. This can happen if the developer has an incorrect understanding of the current storage layout or if there are changes to the contract that inadvertently affect the layout.

The add(slot, i) calculation does not inherently account for Solidity's storage layout rules, such as the packing of multiple variables into a single slot where possible, or the allocation of new slots for larger data types. If the developer is not careful, they might not account for these rules when manually calculating storage slots, leading to the aforementioned collisions.
Furthermore, the loop increments the slot index i for each pair of reserves being stored. If the loop does not correctly handle the end condition or if there's an off-by-one error, it could result in writing to a slot that is intended for other data. The risk is compounded when considering future modifications to the contract that might introduce new state variables or change the order of existing ones, as such changes could invalidate the assumptions made about free storage slots.

Impact

The impact of this vulnerability is significant. Storage collisions can corrupt the state of the contract, leading to unpredictable behavior. This could manifest as incorrect calculations, loss of critical data, or even making the contract unusable. In a financial context, such as with a contract managing reserves, the consequences could include the loss of funds or the inability to accurately track and manage financial positions.

Tools Used

Manual

Recommendations

Implement and use rigorous checks to ensure that calculated storage slots do not overlap with slots used for other data. Alternatively, use Solidity's automatic storage handling where possible to reduce the risk of manual errors.

Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Too generic
Assigned finding tags:

Informational/Invalid

Support

FAQs

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