Beanstalk: The Finale
Beanstalk
DeFiHardhatFoundry
250,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

`DIAMOND_STORAGE_POSITION`'s storage slot can be prone to pre-image attacks

dimah7

Summary

In LibDiamond.sol library, the declared:

bytes32 constant DIAMOND_STORAGE_POSITION = keccak256("diamond.standard.diamond.storage");

The hashed value serves as a unique identifier for the storage slot that holds the state of the proxy, which can be seen here in LibDiamond::diamondStorage:

function diamondStorage() internal pure returns (DiamondStorage storage ds) {
bytes32 position = DIAMOND_STORAGE_POSITION;
assembly {
ds.slot := position
}
}

Same applies for the tractor storage:

library LibTractor {
// 0x7efbaaac9214ca1879e26b4df38e29a72561affb741bba775ce66d5bb6a82a07
// bytes32 constant TRACTOR_STORAGE_POSITION = keccak256("diamond.storage.tractor");

However slots manually constructed using a string combined with keccak256 for hashing can be prone to storage slot collision as the hash value is known. This can allow potential attackers to relocate the storage slot location, using the same keccak256 method with additional crafted malicious payload.

Impact

  • Impact: High, as this can lead to storage collisions or in worst case an attacker can may find a potential path to these slots using keccak256 for hashing. Based on protocol's complexity, the damage it would cost is huge.

  • Likelihood: Low

  • Overall: Medium

Tools Used

Manual Review

Recommendations

It might be best, to subtract with by 1, so that the pre-image would not be easily attainable. For example OpenZeppelin uses that technique:

in BeaconProxy.sol: The beacon address is stored in storage slot uint256(keccak256('eip1967.proxy.beacon')) - 1, so that it doesn't conflict with the storage layout of the implementation behind the proxy.

in ERC1967Upgrade.sol:

// This is the keccak-256 hash of "eip1967.proxy.rollback" subtracted by 1
bytes32 private constant _ROLLBACK_SLOT = 0x4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd9143;
​
​
* @dev Storage slot with the address of the current implementation.
* This is the keccak-256 hash of "eip1967.proxy.implementation" subtracted by 1, and is
* validated in the constructor.
*/
bytes32 internal constant _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
​
...

Also here is a one reference link: https://forum.openzeppelin.com/t/calculation-of-proxy-contract-storage-slot/30745

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

dimah7 Submitter
12 months ago
inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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