From d96ea29d7f7d873ec77701bccea303f06e229ed7 Mon Sep 17 00:00:00 2001 From: alrevuelta Date: Thu, 27 Jun 2024 13:58:55 +0200 Subject: [PATCH 1/5] fix(rln): fix nullifier log vuln --- waku/waku_rln_relay/rln_relay.nim | 36 ++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/waku/waku_rln_relay/rln_relay.nim b/waku/waku_rln_relay/rln_relay.nim index ab4ba10a6a..623d9b62ef 100644 --- a/waku/waku_rln_relay/rln_relay.nim +++ b/waku/waku_rln_relay/rln_relay.nim @@ -4,7 +4,7 @@ else: {.push raises: [].} import - std/[sequtils, tables, times, deques], + std/[sequtils, tables, times, deques, algorithm], chronicles, options, chronos, @@ -144,7 +144,7 @@ proc updateLog*( try: # check if an identical record exists if rlnPeer.nullifierLog[epoch].hasKeyOrPut(proofMetadata.nullifier, proofMetadata): - # the above condition could be `discarded` but it is kept for clarity, that slashing will + # the above condition could be `discarded` but it is kept for clarity, that slashing will # be implemented here # TODO: slashing logic return ok() @@ -280,7 +280,7 @@ proc validateMessageAndUpdateLog*( return MessageValidationResult.Invalid # insert the message to the log (never errors) - discard rlnPeer.updateLog(msgProof.epoch, proofMetadataRes.get()) + #discard rlnPeer.updateLog(msgProof.epoch, proofMetadataRes.get()) return isValidMessage @@ -308,10 +308,17 @@ proc appendRLNProof*( let proof = rlnPeer.groupManager.generateProof(input, epoch, nonce).valueOr: return err("could not generate rln-v2 proof: " & $error) - msg.proof = proof.encode().buffer return ok() +proc compareKeys(a, b: Epoch): int = + for i in 0..<32: + if a[i] < b[i]: + return -1 + elif a[i] > b[i]: + return 1 + return 0 + proc clearNullifierLog*(rlnPeer: WakuRlnRelay) = # clear the first MaxEpochGap epochs of the nullifer log # if more than MaxEpochGap epochs are in the log @@ -319,10 +326,23 @@ proc clearNullifierLog*(rlnPeer: WakuRlnRelay) = if rlnPeer.nullifierLog.len().uint <= rlnPeer.rlnMaxEpochGap: return + echo "elements in nullifier log" + for i in rlnPeer.nullifierLog.keys().toSeq(): + echo i let countToClear = rlnPeer.nullifierLog.len().uint - rlnPeer.rlnMaxEpochGap trace "clearing epochs from the nullifier log", count = countToClear - let epochsToClear = rlnPeer.nullifierLog.keys().toSeq()[0 ..< countToClear] + + #let epochsToClear = rlnPeer.nullifierLog.keys().toSeq()[0 ..< countToClear] + #let epochsToClear = rlnPeer.nullifierLog.keys().toSeq().sort(compareKeys)[0 ..< countToClear] + + var epochsToClear = rlnPeer.nullifierLog.keys().toSeq() + epochsToClear.sort(compareKeys) + epochsToClear = epochsToClear[0 ..< countToClear] + + info "ordered: ", ordered=epochsToClear + for epoch in epochsToClear: + info "removing epoch", epoch = epoch rlnPeer.nullifierLog.del(epoch) proc generateRlnValidator*( @@ -358,6 +378,12 @@ proc generateRlnValidator*( payload = string.fromBytes(message.payload) case validationRes of Valid: + let proofMetadataRes = msgProof.extractMetadata() + if proofMetadataRes.isErr(): + # dirty should never happen + return pubsub.ValidationResult.Reject + # insert the message to the log (never errors) + discard wakuRlnRelay.updateLog(msgProof.epoch, proofMetadataRes.get()) trace "message validity is verified, relaying:", proof = proof, root = root, From 75167ac23c74e858067ba6d553a91892b885d17f Mon Sep 17 00:00:00 2001 From: alrevuelta Date: Thu, 27 Jun 2024 16:00:26 +0200 Subject: [PATCH 2/5] more changes --- waku/waku_rln_relay/rln_relay.nim | 39 +++++++++---------------------- 1 file changed, 11 insertions(+), 28 deletions(-) diff --git a/waku/waku_rln_relay/rln_relay.nim b/waku/waku_rln_relay/rln_relay.nim index 623d9b62ef..f737066e83 100644 --- a/waku/waku_rln_relay/rln_relay.nim +++ b/waku/waku_rln_relay/rln_relay.nim @@ -279,8 +279,10 @@ proc validateMessageAndUpdateLog*( if proofMetadataRes.isErr(): return MessageValidationResult.Invalid - # insert the message to the log (never errors) - #discard rlnPeer.updateLog(msgProof.epoch, proofMetadataRes.get()) + # insert the message to the log (never errors) only if the + # message is valid. + if isValidMessage == MessageValidationResult.Valid: + discard rlnPeer.updateLog(msgProof.epoch, proofMetadataRes.get()) return isValidMessage @@ -322,28 +324,15 @@ proc compareKeys(a, b: Epoch): int = proc clearNullifierLog*(rlnPeer: WakuRlnRelay) = # clear the first MaxEpochGap epochs of the nullifer log # if more than MaxEpochGap epochs are in the log - # note: the epochs are ordered ascendingly - if rlnPeer.nullifierLog.len().uint <= rlnPeer.rlnMaxEpochGap: - return - - echo "elements in nullifier log" - for i in rlnPeer.nullifierLog.keys().toSeq(): - echo i - let countToClear = rlnPeer.nullifierLog.len().uint - rlnPeer.rlnMaxEpochGap - trace "clearing epochs from the nullifier log", count = countToClear - - #let epochsToClear = rlnPeer.nullifierLog.keys().toSeq()[0 ..< countToClear] - #let epochsToClear = rlnPeer.nullifierLog.keys().toSeq().sort(compareKeys)[0 ..< countToClear] + let currentEpoch = fromEpoch(rlnPeer.getCurrentEpoch()) - var epochsToClear = rlnPeer.nullifierLog.keys().toSeq() - epochsToClear.sort(compareKeys) - epochsToClear = epochsToClear[0 ..< countToClear] + for epoch in rlnPeer.nullifierLog.keys(): + let epochInt = fromEpoch(epoch) - info "ordered: ", ordered=epochsToClear - - for epoch in epochsToClear: - info "removing epoch", epoch = epoch - rlnPeer.nullifierLog.del(epoch) + # clean all epochs that are +- rlnMaxEpochGap from the current epoch + if (currentEpoch+rlnPeer.rlnMaxEpochGap) < epochInt and epochInt < (currentEpoch-rlnPeer.rlnMaxEpochGap): + trace "clearing epochs from the nullifier log", cleanedEpoch = epochInt + rlnPeer.nullifierLog.del(epoch) proc generateRlnValidator*( wakuRlnRelay: WakuRLNRelay, spamHandler = none(SpamHandler) @@ -378,12 +367,6 @@ proc generateRlnValidator*( payload = string.fromBytes(message.payload) case validationRes of Valid: - let proofMetadataRes = msgProof.extractMetadata() - if proofMetadataRes.isErr(): - # dirty should never happen - return pubsub.ValidationResult.Reject - # insert the message to the log (never errors) - discard wakuRlnRelay.updateLog(msgProof.epoch, proofMetadataRes.get()) trace "message validity is verified, relaying:", proof = proof, root = root, From a21aacddaa43d34e5ec9ca67c26d73d71bb2273a Mon Sep 17 00:00:00 2001 From: alrevuelta Date: Thu, 27 Jun 2024 16:02:51 +0200 Subject: [PATCH 3/5] remove unused stuff --- waku/waku_rln_relay/rln_relay.nim | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/waku/waku_rln_relay/rln_relay.nim b/waku/waku_rln_relay/rln_relay.nim index f737066e83..b0107929fb 100644 --- a/waku/waku_rln_relay/rln_relay.nim +++ b/waku/waku_rln_relay/rln_relay.nim @@ -4,7 +4,7 @@ else: {.push raises: [].} import - std/[sequtils, tables, times, deques, algorithm], + std/[sequtils, tables, times, deques], chronicles, options, chronos, @@ -313,14 +313,6 @@ proc appendRLNProof*( msg.proof = proof.encode().buffer return ok() -proc compareKeys(a, b: Epoch): int = - for i in 0..<32: - if a[i] < b[i]: - return -1 - elif a[i] > b[i]: - return 1 - return 0 - proc clearNullifierLog*(rlnPeer: WakuRlnRelay) = # clear the first MaxEpochGap epochs of the nullifer log # if more than MaxEpochGap epochs are in the log From cba25aec1496deaa9600e3f7701af75a56cfa6e5 Mon Sep 17 00:00:00 2001 From: alrevuelta Date: Thu, 27 Jun 2024 16:46:35 +0200 Subject: [PATCH 4/5] some fixes --- waku/waku_rln_relay/rln_relay.nim | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/waku/waku_rln_relay/rln_relay.nim b/waku/waku_rln_relay/rln_relay.nim index b0107929fb..869886db82 100644 --- a/waku/waku_rln_relay/rln_relay.nim +++ b/waku/waku_rln_relay/rln_relay.nim @@ -318,13 +318,17 @@ proc clearNullifierLog*(rlnPeer: WakuRlnRelay) = # if more than MaxEpochGap epochs are in the log let currentEpoch = fromEpoch(rlnPeer.getCurrentEpoch()) + var epochsToRemove: seq[Epoch] = @[] for epoch in rlnPeer.nullifierLog.keys(): let epochInt = fromEpoch(epoch) # clean all epochs that are +- rlnMaxEpochGap from the current epoch - if (currentEpoch+rlnPeer.rlnMaxEpochGap) < epochInt and epochInt < (currentEpoch-rlnPeer.rlnMaxEpochGap): - trace "clearing epochs from the nullifier log", cleanedEpoch = epochInt - rlnPeer.nullifierLog.del(epoch) + if (currentEpoch+rlnPeer.rlnMaxEpochGap) <= epochInt or epochInt <= (currentEpoch-rlnPeer.rlnMaxEpochGap): + epochsToRemove.add(epoch) + + for epochRemove in epochsToRemove: + trace "clearing epochs from the nullifier log", currentEpoch = currentEpoch, cleanedEpoch = epochInt + rlnPeer.nullifierLog.del(epochRemove) proc generateRlnValidator*( wakuRlnRelay: WakuRLNRelay, spamHandler = none(SpamHandler) From 84ef6afc8e02f2b5021ccfe12aa5350a91a67b13 Mon Sep 17 00:00:00 2001 From: alrevuelta Date: Thu, 27 Jun 2024 16:50:55 +0200 Subject: [PATCH 5/5] another fix --- waku/waku_rln_relay/rln_relay.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/waku/waku_rln_relay/rln_relay.nim b/waku/waku_rln_relay/rln_relay.nim index 869886db82..85890a2cd2 100644 --- a/waku/waku_rln_relay/rln_relay.nim +++ b/waku/waku_rln_relay/rln_relay.nim @@ -327,7 +327,7 @@ proc clearNullifierLog*(rlnPeer: WakuRlnRelay) = epochsToRemove.add(epoch) for epochRemove in epochsToRemove: - trace "clearing epochs from the nullifier log", currentEpoch = currentEpoch, cleanedEpoch = epochInt + trace "clearing epochs from the nullifier log", currentEpoch = currentEpoch, cleanedEpoch = fromEpoch(epochRemove) rlnPeer.nullifierLog.del(epochRemove) proc generateRlnValidator*(