Part 2

Summary

The VAULT_LOCATION constant incorrectly calculates storage slots by subtracting 1 from a keccak256 hash result before re-hashing. This deviates from Solidity's standard storage isolation patterns, creating risks of storage collisions where vault data could overwrite/be overwritten by unrelated contract variables.

Vulnerability Details

Faulty Code

Technical Breakdown

1. Intended Behavior:
   Storage slots for vaults should be isolated using:

   keccak256("fi.zaros.market-making.Vault")

   This follows Solidity's convention for deterministic, collision-resistant namespacing.

2. Actual Behavior:

   • Step 1: Compute keccak256("fi.zaros...") → 32-byte hash H

   • Step 2: Convert H to uint256 → H_num

   • Step 3: Compute H_num - 1 → Alters hash value

   • Step 4: Re-encode as bytes32 → Creates corrupted base slot H'

   • Step 5: Final slot = keccak256(abi.encode(H'))

3. Key Issue:

   The -1 operation shifts the base slot to an unpredictable location, breaking storage isolation guarantees.

Impact

• Data Corruption: Overwritten vault balances/debt calculations could lead to:
- Incorrect asset valuations
- Unauthorized withdrawals
- Protocol insolvency

• Upgrade Risks: Future contract upgrades using standard slot patterns will collide with existing vault data

Example Scenario:
If another contract component uses:

The vulnerable VAULT_LOCATION becomes SAFE_SLOT - 1, causing adjacent storage allocation. Writing to a vault could overwrite data in SAFE_SLOT.

Tools Used

Manual
Solidity Docs and Blogs

Recommendations

Immediate Fix

Remove the -1 adjustment:

Preventative Measures

1. Storage Layout Tests:
   Add Foundry tests to verify slot calculations:

   function test_VAULT_LOCATION() public {

   bytes32 expected = keccak256("fi.zaros.market-making.Vault");

   assertEq(VAULT_LOCATION, expected);

   }