Project
One World
NFTDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Missing Return Value Check for Set Operations in Currency Management

yourguyd3v

Summary

In the addCurrency and removeCurrency functions, the contract does not check the return value of the _whitelistedCurrencies.add and _whitelistedCurrencies.remove operations. This could lead to unexpected behavior if these operations fail, as no revert or indication of failure would be provided to the caller.

Vulnerability Details

Issue: The functions _whitelistedCurrencies.add and _whitelistedCurrencies.remove return a boolean value indicating success, but the return value is currently ignored. Ignoring the return value may lead to the assumption that the currency was successfully added or removed when, in fact, it was not.

Root Cause: Lack of a return value check after invoking operations on the _whitelistedCurrencies set.

#L40
#L54

Impact

If the add or remove the operation fails silently, the contract might operate under the assumption that a currency has been successfully added to or removed from the whitelist. This could lead to inconsistencies within the system, potentially allowing or disallowing certain currencies in ways not intended.

Tools Used

Manual code review

Recommendations

Add checks for the boolean return values of _whitelistedCurrencies.add and _whitelistedCurrencies.remove to ensure successful execution:

function addCurrency(
address currency
) external override onlyRole(ADMIN_ROLE) {
if (currency == address(0))
revert CurrencyManagerError("Cannot be null address");
if (_whitelistedCurrencies.contains(currency))
revert CurrencyManagerError("Already whitelisted");
- _whitelistedCurrencies.add(currency);
+ require(_whitelistedCurrencies.add(currency), "Currency addition failed");
​
emit CurrencyWhitelisted(currency);
}
function removeCurrency(
address currency
) external override onlyRole(ADMIN_ROLE) {
if (!_whitelistedCurrencies.contains(currency))
revert CurrencyManagerError("Not whitelisted");
- _whitelistedCurrencies.remove(currency);
+ require(_whitelistedCurrencies.remove(currency), "Currency removal failed");
​
​
emit CurrencyRemoved(currency);
}
Updates

Lead Judging Commences

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

Support

FAQs

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