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

_isWhiteListed might return wrong status

0xspacepirate

Summary

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

The Bridge::_isWhiteListed function might return wrong status due to wrong return statement.

Vulnerability Details

The statement !_whiteListEnabled || _whiteList[collection] inside Bridge::_isWhiteListed is incorrect because if _whiteListEnabled = false
then it will always return true even if the collection is NOT whitelisted:

function _isWhiteListed(
address collection
) internal view returns (bool) {
return !_whiteListEnabled || _whiteList[collection]; // audit - incorrect, when !_whiteListEnabled is false the result will always be true
}

As a result one of the whitelist checks at the beginning of the function Bridge::depositTokens is bypassed and Bridge::isWhiteListed may return incorrect result.

Impact

Impact: High
Likelihood: High

Proof of Concept

An user whose collection is NOT whitelisted calls Bridge::depositTokens while _whiteListEnabled = false. Thus, bypassing the check in Bridge::depositTokens:

if (!_isWhiteListed(collectionL1)) {
revert NotWhiteListedError();
}

Consequently, the user would be able to deposit tokens without being whitelisted.

Tools Used

Manual Review

Recommendations

Option 1

Remove !_whiteListEnabled from the statement:

function _isWhiteListed(
address collection
) internal view returns (bool) {
- return !_whiteListEnabled || _whiteList[collection];
+ return _whiteList[collection];
}

Option 2

An alternative is to keep the !_whiteListEnabled and change the OR(||) to AND(&&), and change '!_whiteListEnabled' to _whiteListEnabled:

function _isWhiteListed(
address collection
) internal view returns (bool) {
- return !_whiteListEnabled || _whiteList[collection];
+ return _whiteListEnabled && _whiteList[collection];
}

Either options are fine, depends on how the sponsors want the protocol to work.

Option 1 - return whether the collection is whitelisted at all.

Option 2 - return whether the whitelisting is enabled and also whether the collection exists too.

In my opinion, it should be as in option 1.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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