NFTBridge
60,000 USDC
View results
Submission Details
Severity: low
Invalid

Whitelisting is available even when _whiteListEnabled is false

Summary

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

Whitelisting is available even when _whiteListEnabled is false due to incorrect if statement.

Vulnerability Details

In Bridge::_whiteListCollection if the _whiteListEnabled is not enabled the users are still able to whitelist their collections at a contract state level:

function _whiteListCollection(address collection, bool enable) internal {
if (enable && !_whiteList[collection]) { // missing check for whether whitelisting is enabled at all at a contract state level
...
}
...
}

Since the check is missing, this would enable users to whitelist collections even when it's not allowed through Bridge::withdrawTokens.

Impact

Impact: High

Likelihood: Medium

Proof of Concept

User calls Bridge::withdrawTokens where the address of collectionL1 must be equal to address(0x0) and the CollectionType is CollectionType.ERC721 so the logic calls the Bridge::_whiteListCollection inside Bridge::withdrawTokens.

Another entry point for the Bridge::_whiteListCollection is the Bridge::whiteList whose access control is restricted to the owner. Still if the owner has NOT enabled the
_whiteListEnabled through Bridge::enableWhiteList then even they should NOT be able to whitelist, otherwise what is the point of having a _whiteListEnabled.

Tools Used

Manual Review

Recommendations

Add a missing parameter check _whiteListEnabled in the Bridge::_whiteListCollection:

function _whiteListCollection(address collection, bool enable) internal {
- if (enable && !_whiteList[collection]) {
+ if (enable && !_whiteList[collection] && _whiteListEnabled) {
bool toAdd = true;
uint256 i = 0;
while(i < _collections.length) {
if (collection == _collections[i]) {
toAdd = false;
break;
}
i++;
}
if (toAdd) {
_collections.push(collection);
}
+ _whiteList[collection] = enable; // <-- audit - we want to whitelist only when -> enable && !_whiteList[collection] && _whiteListEnabled
}
if (!enable) {
_whiteList[collection] = enable;
}
}

Another thing for the sponsors to consider is whether it's a good idea to be able to unwhitelist a collection when the _whiteListEnabled is false because at the moment that is a possibility.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Support

FAQs

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