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

Unnecessary processing and event emission for already whitelisted collections

Summary

Unnecessary processing and event emission for already whitelisted collections.

Vulnerability Details

The _whiteListCollection function performs unnecessary operations when attempting to whitelist an already whitelisted collection. This leads to redundant processing and gas consumption.

function whiteList(address collection, bool enable) external onlyOwner {
_whiteListCollection(collection, enable);
emit CollectionWhiteListUpdated(collection, enable);
}
function _whiteListCollection(address collection, bool enable) internal {
if (enable && !_whiteList[collection]) {
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;
}

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

When a collection that is already whitelisted (_whiteList[collection] == true) is passed to the whiteList function with enable set to true, the function:

  1. Unnecessarily iterates through the _collections array.

  2. Redundantly sets _whiteList[collection] = true.

  3. Emits a CollectionWhiteListUpdated event, even though no change occurred.

Similar issue is in enableWhiteList function.

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

Impact

  1. Increased gas consumption for transactions that attempt to whitelist already whitelisted collections.

  2. Potential confusion due to emitted events that don't represent actual state changes.

Tools Used

Manual review

Recommendations

Add an early return condition in the _whiteListCollection function to skip processing if the collection's whitelist status is not changing:

function _whiteListCollection(address collection, bool enable) internal {
if (_whiteList[collection] == enable) {
return; // Early return if the status isn't changing
}
if (enable) {
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;
}
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.