Reuse of signature to get KYCd after it has been removed

[code423n4]

Lines of code

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

Vulnerability details

Impact

There is no time limit on the validity off KYC digests and users with a removed KYC are not saved.
If a issuer of such a digest is either compromised or if they by mistake issue a digest with a
deadline far into the future a user could reuse the same digest to acquire KYC status after
it has been removed.

Even if no mistake has been made a user could attempt to attack the protocol by quickly purposely
losing KYC by becoming a sanctioned entity and then reusing the same signature to acquire KYC
even though they are a known sanctioned entity.

Proof of Concept

I will outline 2 different scenarios:

1. If deadline is set to a date far into the future, either by mistake or compromised digest issuer,
   a user could continuously re-KYC themselves by using the same digest. Depending on how Ondo records
   KYCd users this could lead to a mismatch between their off-chain records and the actual KYCd users.
   Even if Ondo automatically checks that their off-chain records match the events emitted a user could
   potentially re-kyc every time they wish to use the protocol. As long as Ondo does not realize what it
   going on and removes the permission of the signer of the digest the user could re-KYC.

2. Even if no mistake is made and issuer is not compromised a malicious actor could stage an attack on Ondo.
   If deadline is set to a reasonable number time an attack could KYC with a stolen identity and a clean address.
   After passing KYC the user starts receiving a large amount of assets from Tornado Cash, the user is now holding
   sanctioned funds and could potentially be considered a sanctioned entity. Ondo would reasonably
   run off-chain checks on their KYCd users to check on their status, such a user could be removed before the
   deadline and re-KYC and use the protocol. Ondo would then have allowed a known sanctioned entity that
   had its KYC removed to use its protocol. This could have both reputation and legal consequences.

Tools Used

Manual Review

Recommended Mitigation Steps

Change the kycState mapping to map to an enum instead bool with 3 different states. NON-KYC, KYC and REMOVED-KYC and to add a check in addKYCAddressViaSignature() to check that the user does not have the NON-KYC status. To re-KYC a user the manager would have to first change the status from REMOVED-KYC to NON-KYC it could then apply for KYC as usual.

Adding a time limit to how long a digest is valid is also a good idea to have certainty in that there aren't
users out there that could KYC themselves far into the future. Add additional variables, issue date and end date. Check that those are within certain limits.

[c4-judge]

trust1995 marked the issue as duplicate of #187

[c4-judge]

trust1995 marked the issue as satisfactory

[c4-judge]

trust1995 changed the severity to 2 (Med Risk)