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

Sprint 1 - RLN Audit Report - 0xKitetsu #9

Open
0xkitetsu-dinesh opened this issue Jun 14, 2023 · 1 comment
Open

Sprint 1 - RLN Audit Report - 0xKitetsu #9

0xkitetsu-dinesh opened this issue Jun 14, 2023 · 1 comment

Comments

@0xkitetsu-dinesh
Copy link

0xkitetsu-dinesh commented Jun 14, 2023

yAcademy RLN Review

Review Resources:

Auditors:

Review Summary


RLN - Rate Limiting Nullifier

RLN is a protocol which can be used to limit user messages in anonymous server.

The contracts of the RLN Repo were reviewed over 14 days. The code review was performed between 31st May and 14th June, 2023. The repository was under active development during the review, but the review was limited to the latest commit at the start of the review. This was commit 37073131b9c5910228ad6bdf0fc50080e507166a for the circom-rln repo.

Scope

The scope of the review consisted of the following contracts at the specific commit:

This review is a code review to identify potential vulnerabilities in the code. The reviewers did not investigate security practices or operational security and assumed that privileged accounts could be trusted. The reviewers did not evaluate the security of the code relative to a standard or specification. The review may not have identified all potential attack vectors or areas of vulnerability.

yAcademy and the auditors make no warranties regarding the security of the code and do not warrant that the code is free from defects. yAcademy and the auditors do not represent nor imply to third parties that the code has been audited nor that the code is free from defects. By deploying or using the code, RLN and users of the circuits agree to use the code at their own risk.

Findings Explanation

Findings are broken down into sections by their respective impact:

  • Critical, High, Medium, Low impact
    • These are findings that range from attacks that may cause loss of funds, impact control/ownership of the contracts, or cause any unintended consequences/actions that are outside the scope of the requirements
  • Gas savings
    • Findings that can improve the gas efficiency of the contracts
  • Informational
    • Findings including recommendations and best practices

Critical Findings

C1 - Secret get revealed for certain inputs x

Recommendation

  • constraining a1*x is non-zero
isZero(a1*x).out === 0

High Findings

None.

Medium Findings

None.

Low Findings

L1 - Unconstrained public input.

  • In the withdraw circuit, address is used but not constrained

Technical Details

template Withdraw() {
    signal input identitySecret;
    signal input address;

    signal output identityCommitment <== Poseidon(1)([identitySecret]);
}

Recommendation

Final remarks

Circuits are well-written.Developer using this gadget has to aware of the fact that rln circuits can be used for checking computational correctness of the merkle root and nullifier.Limiting repetition of same message with same messageId and validation of merkle root has to done at contract level.

@curryrasul
Copy link

Hi, thanks for your report!

C1 - Secret get revealed for certain inputs x

Probability of finding preimage of 0 for Poseidon hash is negligible, it's not critical bug; though it may be user error to use 0 value for x, but as it's public input - then it can be checked on the client side - no deal to make it in the circuit.

L1 - Unconstrained public input

Good find! We'll add this "dummy constraint" to the "withdraw" circuit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants