Skip to content

Scurbber crate #32

Open
arminsabouri wants to merge 3 commits into
payjoin:mainfrom
arminsabouri:scurbber-crate
Open

Scurbber crate #32
arminsabouri wants to merge 3 commits into
payjoin:mainfrom
arminsabouri:scurbber-crate

Conversation

@arminsabouri

@arminsabouri arminsabouri commented May 29, 2026

Copy link
Copy Markdown
Contributor

Implementation of payjoin/multiparty-protocol-docs#6.

Key type bytes overlap across PSBT map types. For example, 0x06 is TX_MODIFIABLE in
global but BIP32_DERIVATION in inputs. So filtering requires map context.

Buffers the global map to detect version and counts (INPUT_COUNT/
OUTPUT_COUNT for v2, UNSIGNED_TX parsed via Transaction::consensus_decode
for v0), then streams remaining maps with per-map allowlists for global,
input, and output.

Pair::decode is pub(crate) in psbt-v2 0.3.0; worked around with a
PairDecode extension trait replicating the upstream logic.

@arminsabouri arminsabouri changed the title WIP - Scurbber crate Dont merge / review yet WIP - Scurbber crate May 29, 2026
@arminsabouri arminsabouri force-pushed the scurbber-crate branch 5 times, most recently from 12b7c43 to 32f0d26 Compare June 1, 2026 16:51
@arminsabouri arminsabouri marked this pull request as ready for review June 2, 2026 12:17
@arminsabouri arminsabouri changed the title WIP - Scurbber crate Scurbber crate Jun 2, 2026
Comment thread crates/scrubber/Cargo.toml Outdated
prop-tests = []

[dependencies]
psbt-v2 = "0.3.0" No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no newline at the end

Comment thread crates/scrubber/src/scrub.rs Outdated
}
out.push(0x00);

for map_idx in 0..(n_inputs + n_outputs) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n_inputs + n_outputs overflows, both are attacker-controlled u64 (VarInt counts, up to u64::MAX in 9 bytes). with INPUT_COUNT=u64::MAX, OUTPUT_COUNT=1 it panics under overflow-checks (dev/the nix check profile) and wraps to 0 in release. suggest checked_add → InvalidGlobal?

[features]
default = ["unit-tests", "prop-tests"]
unit-tests = []
prop-tests = []

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems good to have a proptest to assert no out-of-allowlist field survives

Key type bytes overlap across PSBT map types. For example, 0x06 is TX_MODIFIABLE in
global but BIP32_DERIVATION in inputs. So filtering requires map context.

Buffers the global map to detect version and counts (INPUT_COUNT/
OUTPUT_COUNT for v2, UNSIGNED_TX parsed via Transaction::consensus_decode
for v0), then streams remaining maps with per-map allowlists for global,
input, and output.

Pair::decode is pub(crate) in psbt-v2 0.3.0; worked around with a
PairDecode extension trait replicating the upstream logic.

@bc1cindy bc1cindy left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants