zBlock-1 / RLN-audit-report

2 stars 6 forks source link

Sprint 1 - RLN Audit Report - 0xKitetsu #9

Open 0xkitetsu-dinesh opened 1 year ago

0xkitetsu-dinesh commented 1 year ago

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 Findings

C1 - Secret get revealed for certain inputs x

Recommendation

isZero(a1*x).out === 0

High Findings

None.

Medium Findings

None.

Low Findings

L1 - Unconstrained public input.

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 commented 1 year ago

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.