Bound reputation decay loop to prevent unbounded iteration and DoS (#601) - #616
Merged
Levi-Ojukwu merged 3 commits intoJul 28, 2026
Conversation
…ion DoS Issue Invoice-Liquidity-Network#601: periods_passed in get_payer_score was unbounded -- a small governance-configurable decay_period_ledgers (e.g. 1) combined with a long-inactive payer could make the decay loop iterate an arbitrarily large number of times, risking out-of-budget failures on every read of that payer's score and blocking fund_invoice/mark_paid. Fix: caps iteration at MAX_REPUTATION_DECAY_PERIODS (1000, new constant in constants.rs) and short-circuits the score to 0 once more periods than that have passed -- any nonzero decay rate has already reduced the score to effectively nothing by then. Also fixes two unrelated pre-existing compile-blocking bugs found while verifying: tests_storage_layout.rs called a nonexistent TestAddress::random() (no Env in scope) instead of this crate's established Address::generate(&env) convention, plus an out-of-range u32 literal; tests_new_features.rs called .unwrap() once on try_submit_invoices_batch's nested Result instead of twice. Tests: 2 new regression tests in test.rs, both calling payer_score() under the real, unmodified default computation budget (not relaxed) -- succeeding at all, without a budget panic, is itself the proof the unbounded-iteration DoS is fixed: - test_reputation_decay_bounded_for_extremely_long_inactivity - test_reputation_decay_bounded_when_decay_period_is_one_ledger (the exact griefing scenario: decay_period_ledgers=1) Note: storage.rs has an identical but dead-code duplicate of get_payer_score, confirmed never called or exported anywhere -- left unfixed as out of scope (not exploitable). Closes Invoice-Liquidity-Network#601
…ion DoS Issue Invoice-Liquidity-Network#601: periods_passed in get_payer_score was unbounded -- a small governance-configurable decay_period_ledgers (e.g. 1) combined with a long-inactive payer could make the decay loop iterate an arbitrarily large number of times, risking out-of-budget failures on every read of that payer's score and blocking fund_invoice/mark_paid. Fix: caps iteration at MAX_REPUTATION_DECAY_PERIODS (1000, new constant in constants.rs) and short-circuits the score to 0 once more periods than that have passed. Also fixes two unrelated pre-existing compile-blocking bugs found while verifying: tests_storage_layout.rs called a nonexistent TestAddress::random() instead of this crate's established Address::generate(&env) convention, plus an out-of-range u32 literal; tests_new_features.rs called .unwrap() once on try_submit_invoices_batch's nested Result instead of twice. Tests: 2 new regression tests in test.rs, both calling payer_score() under the real, unmodified default computation budget -- succeeding without a budget panic is itself the proof the DoS is fixed: - test_reputation_decay_bounded_for_extremely_long_inactivity - test_reputation_decay_bounded_when_decay_period_is_one_ledger Note: storage.rs has an identical but dead-code duplicate of get_payer_score, confirmed never called or exported -- left unfixed as out of scope. Closes Invoice-Liquidity-Network#601
…y-601-unbounded-loop # Conflicts: # contracts/invoice_liquidity/src/constants.rs
|
@KorexOnchain Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #601
Fix
get_payer_scoreininvoice.rscapped atMAX_REPUTATION_DECAY_PERIODS = 1000iterations; beyond that, score short-circuits to 0 instead of iterating further. Matches the acceptance criteria's specified behavior exactly.Also fixed (pre-existing, found while verifying)
tests_storage_layout.rs: called a nonexistentTestAddress::random()API (noEnvin scope); switched to this crate's own establishedAddress::generate(&env)convention. Also fixed an out-of-range u32 literal.tests_new_features.rs:try_submit_invoices_batch's nestedResultwas only unwrapped once instead of twice.Both were blocking
cargo test -p invoice_liquidity --libfrom compiling at all onmain, unrelated to this issue.Note for maintainers
storage.rshas an identical but dead-code duplicate ofget_payer_score-- confirmed via full call-site trace it's never invoked or exported anywhere. Left unfixed (out of scope, not exploitable), flagging for visibility.Tests
Two new regression tests, both calling
payer_score()under the real, default (non-relaxed) computation budget with periods_passed 1.25x-1.5x the cap:test_reputation_decay_bounded_for_extremely_long_inactivitytest_reputation_decay_bounded_when_decay_period_is_one_ledger(decay_period_ledgers=1, the exact griefing scenario in the issue)Succeeding without a budget panic is itself the proof the DoS is fixed.
Acceptance criteria