Signatures can be misused to reverify

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/cash/kyc/KYCRegistry.sol#L79

Vulnerability details

Impact

It is possible to remove the KYC status of any user by using the removeKYCAddresses function. This could be easily overridden by user using the addKYCAddressViaSignature function as shown below

Proof of Concept

1. User KYC is approved and he is provided with a signature signed by an address with the role kycGroupRoles[kycRequirementGroup]

2. User claims his KYC approved status using addKYCAddressViaSignature function which marks him kycState[kycRequirementGroup][user] = true

3. One of KYC group role realizes that User KYC was not actually proper and decides to remove his KYC status using removeKYCAddresses function

4. This sets kycState[kycRequirementGroup][user] = false

function removeKYCAddresses(
    uint256 kycRequirementGroup,
    address[] calldata addresses
  ) external onlyRole(kycGroupRoles[kycRequirementGroup]) {
    uint256 length = addresses.length;
    for (uint256 i = 0; i < length; i++) {
      kycState[kycRequirementGroup][addresses[i]] = false;
    }
    emit KYCAddressesRemoved(msg.sender, kycRequirementGroup, addresses);
  }

5. But since the signature provided in Step 1 is still not expired so User simply recalls addKYCAddressViaSignature function to again set kycState[kycRequirementGroup][user] = true

function addKYCAddressViaSignature(
    uint256 kycRequirementGroup,
    address user,
    uint256 deadline,
    uint8 v,
    bytes32 r,
    bytes32 s
  ) external {
....
kycState[kycRequirementGroup][user] = true;

    emit KYCAddressAddViaSignature(
      msg.sender,
      user,
      signer,
      kycRequirementGroup,
      deadline
    );
  }

Recommended Mitigation Steps

Mark the signature as used

[c4-judge]

trust1995 marked the issue as duplicate of #187

[c4-judge]

trust1995 marked the issue as satisfactory