curryrasul
commented
1 year ago Hi, thanks for your report!
Unused public inputs could be optimized out
Good find! We'll add this to our circuit.
Different bit size for messageLimit between circuits and contracts
Good find! We'll add range check to the contract!
Unneccary import and inheritance of ownable inside RLN contract
Thanks! We should remove Ownable
The penalty enforced on the user doesn't scale with the magnitude of their spam
You cannot spam more than once. As soon as user exceed the limit, they can be slashed immediately (at least on the node level)
yAcademy Rate-Limit Nullifier Review
Auditors:
Table of Contents
messageLimitbetween circuits and contractsReview Summary
Rate-Limit Nullifier RLN intends to develop a layer for spam resistance in anonymous networks. As a first step users are required to register themselves and stake, onchain. Then users can continue to send messages as they do in anonymous networks, without any onchain interaction. However, if they go above their limit, anyone can prove using ZK proof, that they did so and slash their stake. RLN V1 was already in production, the current audit is for its V2 version V2 introduced the novel feature of message limit per epoch, allowing users to send multiple messages per epoch
The circuits and contracts, both were reviewed over a total of 14 days. The code review was performed by 1 auditor between 31st May and 14th June 2023.
Circum circuits are present at circom-rln, and latest commit
37073131b9c5910228ad6bdf0fc50080e507166awas considered for review. Smart contracts are present at rln-contracts and latest commitdfcdb2beff91583d02636570242cfdd8469b61c1was considered for review.Scope
The scope of the review consisted of the following circuits and contracts at the specific commit:
circom-rln
rln-contracts
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, TODO_protocol_name and users of the contracts agree to use the code at their own risk.
Code Evaluation Matrix (Contracts)
Code Evaluation Matrix (Circuits)
Findings Explanation
Findings are broken down into sections by their respective impact:
Critical Findings
None.
High Findings
None.
Medium Findings
[M-1] Effective stake at risk is only fees and not the complete stake as intended
RLN contracts enforce users to stake and register themselves before being part of the network. The idea here is if a user misbehaves and spams the network, anyone can slash them in return for their stake. However, since networks are anonymous, users can slash themselves using another identity and only lose fees.
Impact
Users lose only part of the stake for their misbehavior
Recommendation
Consider using different incentive mechanism for slash.
Low Findings
[L-1] Unused public inputs could be optimized out
Withdraw circuit has address as public input with the intention of constraining address in proofs to prevent frontrunning. However, it's not explicitly constrained.
Impact
Depending on the library used, this could lead to being optimized and not included in constraints, exposing the user's stake.
Recommendation
Consider constraining address explicitly
[L-2] Different bit size for
messageLimitbetween circuits and contractsRLN.sol allows messageLimit to scale upto 2 256 -1, however, circuits enforce it upto 2 16 -1 only.
Impact
If users stake with huge amount such that they satisfy the constraint of amount / MINIMAL_DEPOSIT >= 2 ** 16, their stake can not be slashed anymore
Recommendation
Consider adding an explicit check on bit size inside contracts as well.
Quality Findings
[Q-1] Unneccary import and inheritance of ownable inside RLN contract
Consider removing the ownable inheritance
Informational Findings
[I-1] By sending the same message continuously, users can still spam the network
It is true that if the users change their msg and go above the msg limit, they expose their secret, however, they can still send the same messages continuously. The protocol team cleared that this is not an issue, since nodes will filter the same msg out.
[I-2] The penalty enforced on the user doesn't scale with the magnitude of their spam
There is no difference between a penalty for spamming once or spamming x1000 times. Hence the cost of attack does not scale with the impact All attacker needs to do is do a minimum deposit and then he/she can spam the network with any number of messages at once, and all they are penalized for is a minimum deposit. This incentive structure puts an innocent and malicious rule breaker in the same bucket.
Final remarks
No critical or high issues were found. For the medium one as well, we were informed that the protocol team is already aware of it. Overall, the contracts and circuits are well-designed.