DeFiLayer 1Layer 2
14,723 OP
View results
Submission Details
Severity: high
Invalid

Incorrect Storage Slot Calculation for profit_max_unlock_time

Summary

The contract calculates the storage slot for the profit_max_unlock_time parameter using the expression :

keccak256(abi.encode(PERIOD_SLOT))

instead of directly using the slot number as a bytes32 value. This miscalculation causes the contract to read from or write to an incorrect storage location, potentially leading to erroneous behavior in price updates and reward calculations.

Vulnerability Details

In Solidity, fixed storage slots are accessed directly using the slot number. For a mapping or dynamic data structure, one might use keccak256 hashing to compute a storage slot. However, when the storage location is fixed (as with the profit_max_unlock_time variable stored at slot 37), the proper approach is to use:

bytes32(uint256(PERIOD_SLOT))

This converts the slot number directly into a bytes32 value without applying any hash function.

POC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.18;
import "forge-std/Test.sol";
import {ScrvusdVerifierV2} from "../src/ScrvusdVerifierV2.sol";
import {IBlockHashOracle} from "../src/oracles/IPinlinkOracle.sol";
contract StorageSlotPoC is Test {
// Deploy a dummy block hash oracle and scrvusd oracle addresses for testing
address constant dummyBlockHashOracle = address(0x123);
address constant dummyScrvusdOracle = address(0x456);
function test_incorrectStorageSlot() public {
// Deploy our verifier contract
ScrvusdVerifierV2 verifier = new ScrvusdVerifierV2(dummyBlockHashOracle, dummyScrvusdOracle);
// Simulate a proof with expected parameters (not a full valid proof, just for illustration)
// In practice, this would be a valid RLP-encoded proof that when processed,
// would try to extract the profit_max_unlock_time from the storage slot.
bytes memory dummyProof = hex"f84c8080808080"; // Dummy proof bytes
// The call should revert or return an unexpected period value because the wrong storage slot is used.
try verifier.verifyPeriodByStateRoot(100, dummyProof) returns (bool result) {
// In a real test, result would not be as expected.
emit log_named_uint("Extracted period", 0);
} catch {
emit log("Transaction reverted due to incorrect storage slot calculation");
}
}

}

Impact

The contract may retrieve an incorrect period value (profit_max_unlock_time), leading to miscalculations in updating the vault price parameters. This could result in inaccurate reward or profit distributions.

Tools Used

Recommendations

Replace :

keccak256(abi.encode(PERIOD_SLOT))

With :

bytes32(uint256(PERIOD_SLOT))

This change ensures that the contract references the correct storage slot directly.

Updates

Lead Judging Commences

0xnevi Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-storage-key-compute-wrong

See primary comments in issue #23

Support

FAQs

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