Repository navigation
bound the peeling decoder on forged tables - #12
Conversation
Adds the paper's two decode guards: a peeled key must hash to the cell it was read from, and at most capacity elements are emitted. Honest sums never trip them; a forged contribution now costs the recipient at most capacity peels instead of running until the counters cycle. The table parameters record the capacity, which the paper's theorem already treats as a scheme parameter.
noot
changed the title
There was a problem hiding this comment.
🟡 Changes recommended
The capacity check in src/mse.rs casts usize to i64, allowing oversized values such as usize::MAX to pass on 64-bit targets and violate the counter no-wrap constraint.
1 open issue
What changed in this PR
This PR hardens the multiset peeling decoder against forged tables by checking key-to-cell consistency and limiting emitted elements to the configured capacity.
Changes:
- Records capacity in
MseParamsand passes the channel’s capacity into it. - Adds decoder guards and exposes their failures through
ChannelError::MalformedEncoding. - Adds rejection tests, updates constructor calls, and documents the bounded decoding behavior.
| File | Description |
|---|---|
tests/rs_mode_e2e.rs |
Supplies capacity when constructing multiset parameters. |
tests/mse_e2e.rs |
Updates test parameters with explicit capacities. |
src/mse.rs |
Adds capacity, decoder guards, error variants, and rejection tests. |
src/channel.rs |
Passes capacity and maps guard failures to channel errors. |
README.md |
Documents malformed-table errors and the peel limit. |
Review effort: Balanced
| assert!(delta >= 1, "δ must be ≥ 1"); | ||
| assert!(capacity >= 1, "capacity must be ≥ 1"); | ||
| assert!( | ||
| (capacity as i64) < T, |
There was a problem hiding this comment.
good catch, fixed: now compares as u64 so a huge capacity can't wrap negative
ameba23
left a comment
ameba23
left a comment
There was a problem hiding this comment.
💯
I think we should merge this regardless of whether #8 is merged as the error handling is a bit better.
I didnt measure performance impact but this adds an extra sha256 per decoded element.
One thing i find a bit confusing not related to this PR - i think src/mse.rs should be iblt.rs. I'm guessing its named mse because it was added first and prony later, but in my understanding both are multi-set encodings.
| ) -> Self { | ||
| assert!(gamma >= 2, "γ must be ≥ 2 (Theorem 3 needs γ ≥ 2)"); | ||
| assert!(delta >= 1, "δ must be ≥ 1"); | ||
| assert!(capacity >= 1, "capacity must be ≥ 1"); |
There was a problem hiding this comment.
i guess we could use NonZeroUsize but this matches the how delta is handled to lets keep it like this.
Adds the two guards the paper's decoder has: a peeled key must hash to the cell it was read from, and at most
capacityelements are emitted.MseParamsrecords the capacity, the channel passes the ρ it already receives, and the two cases surface asChannelError::MalformedEncoding(MseError::{OverCapacity, KeyMismatch}).paper divergence
In "Multiset Encodings from Invertible Bloom Lookup Tables", decode checks before every peel that the cell's key indeed hashes to that cell and that fewer than ρ elements have been output, returning ⊥ otherwise; the text says this does not affect honest encodings but bounds decode to ρ+1 iterations on every input. The correctness proof in the appendix relies on honest sums never tripping either check. The code had neither, so a single corrupt client could submit a forged table and make the recipient peel garbage until the counters cycled. This is a denial of service against the recipient only; the decoder sees the decrypted sum, so nothing per client leaks.
Overlaps with the draft in #8, which adds the same guards as part of the parameter alignment. If that lands first this can be closed.
verification
All tests pass. New unit tests: a table whose count-one cell holds a shifted key is rejected on the first peel, and capacity plus one honest elements are refused.