ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Storage conflicts in `Bridge` contract that could corrupt the contract's state.

0xrolko

Relevant GitHub Links

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/UUPSProxied.sol#L14

Summary

Poor implementation of the openzeppelin UUPSUpgradeable library, which will lead to a storage conflict during the update of th Bridge contract.

Vulnerability Details

The contract Bridge is a contract which can be updated because it inherits from the UUPSUpgradeable library of openzeppelin. however this library requires certain obligations as explained in this documentation: https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps.

Indeed, the __gap storage variable, whose purpose is to reserve a storage space to ensure compatibility with future versions of the contract during updates, has not been implemented. So adding new storage variables to the Bridge contract during the update will overwrite the existing storage locations, breaking the logic of the contract and leading to vulnerabilities.

As we can se down here there is no declaration of __gap storage variable into the proxy contract:

contract UUPSOwnableProxied is Ownable, UUPSUpgradeable {
​
// Mapping for implementations initialization.
mapping(address implementation => bool initialized) _initializedImpls;
​
// onlyInit
modifier onlyInit() {
address impl = _getImplementation();
require(!_initializedImpls[impl], "Already init");
_initializedImpls[impl] = true;
​
_;
}
​
/**
@notice Only owner should be able to upgrade.
*/
function _authorizeUpgrade(
address
)
internal
override
onlyOwner
{ }
​
/**
@notice Ensures unsupported function is directly reverted.
*/
fallback()
external
payable
{
revert NotSupportedError();
}
​
/**
@notice Ensures no ether is received without a function call.
*/
receive()
external
payable
{
revert NotPayableError();
}
}

Impact

Corruption of Bridge contract logic caused by storage collision during the update.

Tools Used

Manual Analysis.

Recommendations

contract UUPSOwnableProxied is Ownable, UUPSUpgradeable {
​
// Mapping for implementations initialization.
mapping(address implementation => bool initialized) _initializedImpls;
​
// onlyInit
modifier onlyInit() {
address impl = _getImplementation();
require(!_initializedImpls[impl], "Already init");
_initializedImpls[impl] = true;
​
_;
}
​
/**
@notice Only owner should be able to upgrade.
*/
function _authorizeUpgrade(
address
)
internal
override
onlyOwner
{ }
​
/**
@notice Ensures unsupported function is directly reverted.
*/
fallback()
external
payable
{
revert NotSupportedError();
}
​
/**
@notice Ensures no ether is received without a function call.
*/
receive()
external
payable
{
revert NotPayableError();
}
+ uint256[50] private __gap;
}
Updates

Lead Judging Commences

n0kto Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

invalid-upgradeable-storage-gap-known-issue

Known issue: Lightchaser

Support

FAQs

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