Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

one of the whitelist mechanisms doesn't work #29

Closed
code423n4 opened this issue Aug 22, 2021 · 1 comment
Closed

one of the whitelist mechanisms doesn't work #29

code423n4 opened this issue Aug 22, 2021 · 1 comment
Labels
1 (Low Risk) bug Something isn't working duplicate This issue or pull request already exists

Comments

@code423n4
Copy link
Contributor

Handle

gpersoon

Vulnerability details

Impact

The function marketWhitelistCheck of RCTreasury checks the variable marketWhitelist.
However marketWhitelist is never set in the code base so calling marketWhitelistCheck is not useful.

Note: there are 2 whitelist mechanisms:

  1. toggleWhitelist / batchWhitelist
  2. marketWhitelistCheck

This might be the reason why this issue wasn't detected earlier.

Proof of Concept

//https://github.com/code-423n4/2021-08-realitycards/blob/main/contracts/RCTreasury.sol#L269
function marketWhitelistCheck(address _user) external view override returns (bool) {
bytes32 requiredRole = marketWhitelist[msgSender()];
if (requiredRole == bytes32(0)) {
return true;
} else {
return hasRole(requiredRole, _user);
}
}

Tools Used

Recommended Mitigation Steps

Add a function to set marketWhitelist (or remove marketWhitelistCheck)

Add comments to show there are two whitelist mechanisms.
Rename toggleWhitelist / batchWhitelist to toggleUrerWhitelist / batchUserWhitelist

@code423n4 code423n4 added 1 (Low Risk) bug Something isn't working labels Aug 22, 2021
code423n4 added a commit that referenced this issue Aug 22, 2021
@Splidge Splidge added the duplicate This issue or pull request already exists label Aug 23, 2021
@Splidge
Copy link
Collaborator

Splidge commented Aug 23, 2021

Duplicate of #18

@Splidge Splidge marked this as a duplicate of #18 Aug 23, 2021
@Splidge Splidge closed this as completed Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 (Low Risk) bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants