-
Notifications
You must be signed in to change notification settings - Fork 0
Description
yAcademy - Rate Limiting Nullifier Review
Review Resources:
- None beyond the code repositories
Auditors:
Table of Contents
- Review Summary
- Scope
- Findings Explanation
- Critical Findings
- High Findings
- Medium Findings
- Low Findings
- Final remarks
Review Summary
Rate Limiting Nullifier
RLN (Rate-Limiting Nullifier) is a zk-gadget/protocol that enables spam prevention mechanism for anonymous environments.
The circuits of RLN were reviewed over 12 days. The code review was performed between 31st May and 12th 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 37073131b9 for the circom-rln repo.
Scope
The scope of the review consisted of the following circuits at the specific commit:
- rln.circom
- utils.circom
- withdraw.circom
After the findings were presented to the RLN team, fixes were made and included in several PRs.
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 contracts agree to use the code at their own risk.
Code Evaluation Matrix
| Category | Mark | Description |
|---|---|---|
| Access Control | Good | TODO |
| Mathematics | Good | TODO |
| Complexity | Good | TODO |
| Libraries | Average | TODO |
| Decentralization | Good | TODO |
| Code stability | Good | TODO |
| Documentation | Low | TODO |
| Monitoring | Average | TODO |
| Testing and verification | Average | TODO |
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
- Informational
- Findings including recommendations and best practices
Critical Findings
None.
High Findings
None
Medium Findings
None.
Low Findings
1. Low - Under constrained userMessageLimit
In utils.circom, the signal limit is under constrained.
Suggested Solution
template RangeCheck(LIMIT_BIT_SIZE) {
assert(LIMIT_BIT_SIZE < 253);
signal input messageId;
signal input limit;
signal bitCheck[LIMIT_BIT_SIZE] <== Num2Bits(LIMIT_BIT_SIZE)(messageId);
signal limitCheck[LIMIT_BIT_SIZE] <== Num2Bits(LIMIT_BIT_SIZE)(limit);
signal rangeCheck <== LessThan(LIMIT_BIT_SIZE)([messageId, limit]);
rangeCheck === 1;
}
2. Low - Incosistency between contract and the circuit on the number of bits for userMessageLimit
RLN.sol
uint256 messageLimit = amount / MINIMAL_DEPOSIT;
rln.circom
template RLN(DEPTH, LIMIT_BIT_SIZE) {
...
// messageId range check
RangeCheck(LIMIT_BIT_SIZE)(messageId, userMessageLimit);
...
}
component main { public [x, externalNullifier] } = RLN(20, 16);
In RLN.sol, the messageLimit can take upto 2**256 - 1 values whereas messageId & userMessageLimit values in circuits is restricted to 2**16 - 1 .
Recommended solution
- RLN.sol
function register(uint256 identityCommitment, uint256 amount) external {
...
uint256 messageLimit = amount / MINIMAL_DEPOSIT;
require( messageLimit <= type(uint16).max , "Max amount of message limit is 65535");
token.safeTransferFrom(msg.sender, address(this), amount);
...
}
Final remarks
TODO