<!-- canonical: https://0xsimao.com/reports/keyring-ii -->

# Keyring Credentials audit

Three Sigma · Zero-knowledge compliance · 10th July 2024

A private security review of Keyring (zero-knowledge compliance), conducted with Three Sigma and completed on 10th July 2024 over 5 days. 0xSimao disclosed 12 findings: 2 high severity, 1 medium, 3 low and 6 informational.

## What Keyring is

Keyring is zero-knowledge compliance infrastructure: users hold credentials attesting policy eligibility (KYC, jurisdiction, sanctions state), verified on-chain through RSA/PKCS#1-style signature checks without revealing identity, so permissioned pools keep composable liquidity.

## Scope

This second Three Sigma round covered the credential verifier and its key handling, credential creation timestamps and expiry, the fee transfer mechanism, and the blacklist machinery that cuts an entity's addresses off.

## High severity findings

- [Credentials can be manipulated](https://0xsimao.com/findings/keyring-ii-credentials-manipulated)
- [RsaVerifyOptimized sets the size of the key to 1024 bits, which is unsafe](https://0xsimao.com/findings/keyring-ii-verify-1024-bits-unsafe)

## Medium severity findings

- [RsaVerifyOptimized::pkcs1Sha256() modified the original code incorrectly in one instance](https://0xsimao.com/findings/keyring-ii-pkcs1-sha256-modified-original)

## Low severity findings

- [KeyringCoreV2Base::collectFees() should use Address::sendValue() instead of address.transfer()](https://0xsimao.com/findings/keyring-ii-keyring-fees-send-transfer)
- [Missing admin change events](https://0xsimao.com/findings/keyring-ii-missing-admin-change-events)
- [KeyringCoreV2Base could use a 2 step admin transfer mechanism](https://0xsimao.com/findings/keyring-ii-keyring-step-transfer-mechanism)

## Informational findings

- [KeyringCoreV2Base::_createCredential() could use currentTime instead of block.timestamp on the creatBefore check](https://0xsimao.com/findings/keyring-ii-credential-current-timestamp-creat)
- [KeyringCoreV2Base::blacklistEntity() and KeyringCoreV2Base::unblacklistEntity() could check that the addresses are not already set](https://0xsimao.com/findings/keyring-ii-blacklist-entity-unblacklist-addresses)
- [Admin checks in KeyringCoreV2Base could be placed in a modifier to increase readability and reduce code duplication](https://0xsimao.com/findings/keyring-ii-placed-readability-reduce-duplication)
- [KeyringCoreV2Base::registerKey() could check that validTo is equal to or bigger than block.timestamp](https://0xsimao.com/findings/keyring-ii-register-equal-bigger-timestamp)
- [Functions can be marked external when only called from other contracts](https://0xsimao.com/findings/keyring-ii-marked-external-called-contracts)
- [Some natspec comments in KeyringCoreV2Base are outdated](https://0xsimao.com/findings/keyring-ii-natspec-comments-keyring-outdated)

## The report

[Download the report (PDF)](https://0xsimao.com/reports/2024-07-10-keyring-ii.pdf)
