From 68973c716aeb7724327642cb7bf9daa8d302269f Mon Sep 17 00:00:00 2001 From: GracedEternalKingCabbageMan <151803062+GracedEternalKingCabbageMan@users.noreply.github.com> Date: Sat, 3 Oct 2026 14:30:01 +0200 Subject: [PATCH] wasm: refuse unknown fields on every Arca object serde_wasm_bindgen reads a struct by asking the JavaScript object for each field the struct names, so a field it does not name is never seen and deny_unknown_fields has nothing to refuse. A misspelt bound in the verifier's policy (maxExitDelaySecond) was accepted and the bound took its default; a misspelt ceiling in signCsfs's limits (maxUncomitted) was ignored and the ceiling fell back to 0. Every Arca object is now read whole first, every key with it, and then into its type, where deny_unknown_fields applies at every level: the policy and its fee-rate floor, the limits, the message, its source, outputs and inputs. A source is read as a record or as the named form by whether it has `record`, so a stray field is named in the refusal rather than reported as matching no variant. --- SEQUENTIA.md | 8 ++++++-- lwk_wasm/src/ark.rs | 24 ++++++++++++++++++----- lwk_wasm/src/csfs.rs | 30 +++++++++++++++++++++++------ lwk_wasm/tests/node/arca_signers.js | 10 ++++++++++ lwk_wasm/tests/node/ark_records.js | 4 ++++ 5 files changed, 63 insertions(+), 13 deletions(-) diff --git a/SEQUENTIA.md b/SEQUENTIA.md index 67c2ffc0c..268498589 100644 --- a/SEQUENTIA.md +++ b/SEQUENTIA.md @@ -448,7 +448,9 @@ The fork is not published to npm; consumers build `pkg/` with `wasm-pack`. are the transaction's other inputs, `[]` for a coin spent alone. `limits` is `{ feeFloorPerKvb }` for the specification's fee margin or `{ maxUncommitted }` for a ceiling in atoms; without it a rebind must commit - the whole coin. + the whole coin. A message, a source, an output or input, or `limits` with a + field it does not name is refused (`unknown field ...`), so a misspelt field + never quietly takes a default. - `src/ark.rs`: Arca leaves. `arkNewOwnerNonce()`, `arkLeafKeyPath(account, ownerNonce)`, `Signer.arkLeafKey(account, ownerNonce)` and `Signer.arkRestoreKey(account, record)` for the leaf keys; @@ -459,7 +461,9 @@ The fork is not published to npm; consumers build `pkg/` with `wasm-pack`. chain's median time `now` as its last argument (the policy names none, and refuses one), whose verdict is `{ accepted: true, leafId, roundTxid, asset, value, expiries, ... }` or - `{ accepted: false, failed, check, kind, reason }`; and `ArkStore(storage)` + `{ accepted: false, failed, check, kind, reason }`; a policy with a field + it does not name is refused, as a misspelt bound would otherwise take its + default; and `ArkStore(storage)` over a `JsStorage` object, with `putPending`, `putLeaf`, `putRestoredLeaf` and `removeLeaf` keeping the same rules as the native store. A record is its JSON text or its binary form as hex. Asset ids, the token and the genesis hash are display hex at this diff --git a/lwk_wasm/src/ark.rs b/lwk_wasm/src/ark.rs index 06e690393..d27675e1d 100644 --- a/lwk_wasm/src/ark.rs +++ b/lwk_wasm/src/ark.rs @@ -37,6 +37,19 @@ fn generic(e: impl std::fmt::Display) -> Error { Error::Generic(e.to_string()) } +/// Read a plain object from JavaScript into `T`, refusing any field `T` does +/// not name, at every level. +/// +/// `serde_wasm_bindgen` reads a struct by asking the object for each field the +/// struct names, so a field it does not name is never seen and +/// `deny_unknown_fields` has nothing to refuse: a misspelt bound would quietly +/// take its default. The object is read whole first, every key with it, and +/// then into `T`, where `deny_unknown_fields` applies. +pub(crate) fn from_js(value: JsValue) -> Result { + let whole: serde_json::Value = serde_wasm_bindgen::from_value(value)?; + Ok(serde_json::from_value(whole)?) +} + fn refused(e: &RecordError) -> Error { Error::Generic(format!("record refused (kind {}): {e}", e.kind())) } @@ -194,8 +207,8 @@ pub fn ark_parse_record(record: &str) -> Result { #[serde(rename_all = "camelCase", deny_unknown_fields)] struct PolicyDto { operator: String, - /// Refused when present: `now` is given to each call. Named here because - /// an object read from JavaScript ignores fields the struct does not name. + /// Refused when present, with its own message: `now` is given to each + /// call. now: Option, min_notice_seconds: Option, horizon_seconds: Option, @@ -207,7 +220,7 @@ struct PolicyDto { } #[derive(Deserialize)] -#[serde(rename_all = "camelCase")] +#[serde(rename_all = "camelCase", deny_unknown_fields)] struct FeeRateFloorDto { floor_per_kvb: u64, multiple: u64, @@ -319,7 +332,8 @@ impl VerdictDto { /// /// The policy object is `{ operator, minNoticeSeconds?, horizonSeconds?, /// minExitDelaySeconds?, maxExitDelaySeconds?, maxLevels?, minReserveAtoms?, -/// minReserveFeeRate? }`: the operator key the wallet was told (x-only hex) +/// minReserveFeeRate? }`, refusing any field it does not name: the operator key +/// the wallet was told (x-only hex) /// and the bounds, which default to the specification's (a notice of at least /// 36 hours, a first expiry at least 27 days after now for a leaf taken from a /// round, an exit delay of 36 to 48 hours, a path of at most 5 levels, and a @@ -358,7 +372,7 @@ impl ArkVerifier { /// A verifier for `network` with the `policy` object described above. #[wasm_bindgen(constructor)] pub fn new(network: &Network, policy: JsValue) -> Result { - let dto: PolicyDto = serde_wasm_bindgen::from_value(policy)?; + let dto: PolicyDto = from_js(policy)?; if dto.now.is_some() { return Err(generic( "the policy takes no `now`: give the chain's median time to each call", diff --git a/lwk_wasm/src/csfs.rs b/lwk_wasm/src/csfs.rs index bd1535abb..23eb50261 100644 --- a/lwk_wasm/src/csfs.rs +++ b/lwk_wasm/src/csfs.rs @@ -26,6 +26,10 @@ //! `htlc-refund-both` and `leafId` is the id of the leaf the output is, or //! was made from. //! +//! Every object here, the message, its source, outputs and inputs, and the +//! `limits` of `signCsfs`, is refused when it carries a field it does not +//! name, so a misspelt field never quietly takes a default. +//! //! A rebind's `otherInputs` are the transaction's other inputs, `[]` when the //! coin is spent alone; for a reassignment, every other checkpoint. What the //! transaction leaves to whoever broadcasts is reckoned over all of them. @@ -64,7 +68,7 @@ impl Atoms { } #[derive(Deserialize)] -#[serde(rename_all = "camelCase")] +#[serde(rename_all = "camelCase", deny_unknown_fields)] struct OutputDto { asset: String, value: Atoms, @@ -72,6 +76,7 @@ struct OutputDto { } #[derive(Deserialize)] +#[serde(deny_unknown_fields)] struct InputDto { asset: String, value: Atoms, @@ -92,15 +97,28 @@ struct NamedSourceDto { salt: String, } -#[derive(Deserialize)] -#[serde(untagged)] +/// A source is `{ record }` when it names a record, and the named form +/// otherwise; each refuses fields it does not name, which an untagged enum +/// would report only as matching no variant. enum SourceDto { Record(RecordSourceDto), Named(NamedSourceDto), } +impl<'de> Deserialize<'de> for SourceDto { + fn deserialize>(d: D) -> Result { + let v = serde_json::Value::deserialize(d)?; + let source = if v.get("record").is_some() { + serde_json::from_value(v).map(SourceDto::Record) + } else { + serde_json::from_value(v).map(SourceDto::Named) + }; + source.map_err(serde::de::Error::custom) + } +} + #[derive(Deserialize)] -#[serde(tag = "kind", rename_all = "camelCase")] +#[serde(tag = "kind", rename_all = "camelCase", deny_unknown_fields)] enum MessageDto { #[serde(rename_all = "camelCase")] Rebind { @@ -180,7 +198,7 @@ pub(crate) fn parse_limits(limits: JsValue, genesis_hash: BlockHash) -> Result { @@ -196,7 +214,7 @@ pub(crate) fn parse_limits(limits: JsValue, genesis_hash: BlockHash) -> Result Result { - let dto: MessageDto = serde_wasm_bindgen::from_value(message)?; + let dto: MessageDto = crate::ark::from_js(message)?; Ok(match dto { MessageDto::Rebind { source: src, diff --git a/lwk_wasm/tests/node/arca_signers.js b/lwk_wasm/tests/node/arca_signers.js index 199bd6979..94e496cc3 100644 --- a/lwk_wasm/tests/node/arca_signers.js +++ b/lwk_wasm/tests/node/arca_signers.js @@ -167,6 +167,16 @@ assert.throws(() => signer.signCsfs('m/6/0', rebindElsewhere, lwk.csfsDigest(reb /not this wallet's/); assert.throws(() => lwk.csfsDigest({ ...rebind, source: { ...rebind.source, path: 'exit' } }), /unknown rebindable path/); +// A field a message, a source, an output or the limits do not name is refused, +// not ignored: a misspelt ceiling would otherwise take its default. +assert.throws(() => signer.signCsfs('m/6/0', rebind, d, { maxUncomitted: '9999000' }), /unknown field `maxUncomitted`/); +assert.throws(() => lwk.csfsDigest({ ...rebind, source: { ...rebind.source, salts: '07'.repeat(32) } }), /unknown field `salts`/); +assert.throws(() => lwk.csfsDigest({ ...rebind, source: { record: 'ab', leafId: 'a1'.repeat(32) } }), /unknown field `leafId`/); +assert.throws(() => lwk.csfsDigest({ ...rebind, outputs: [{ ...rebind.outputs[0], amount: 5 }] }), /unknown field `amount`/); +assert.throws(() => lwk.csfsDigest({ ...rebind, otherInputs: [{ asset: X, value: 1, vout: 0 }] }), /unknown field `vout`/); +assert.throws(() => lwk.csfsDigest({ ...unroll, tme: 5 }), /unknown field `tme`/); +assert.throws(() => lwk.csfsDigest({ ...release, connectorAsset: X }), /unknown field `connectorAsset`/); + // What a reassignment leaves is reckoned over every input (review R4, F9): // a checkpoint of 100,000 atoms over outputs of 150,000 and 10,000. const spk = (b) => '5120' + b.repeat(32); diff --git a/lwk_wasm/tests/node/ark_records.js b/lwk_wasm/tests/node/ark_records.js index c00872b4f..8f1ed4568 100644 --- a/lwk_wasm/tests/node/ark_records.js +++ b/lwk_wasm/tests/node/ark_records.js @@ -86,6 +86,10 @@ const shallow = verifier({ maxLevels: 1 }).verifyLeaf(dr.json, deep.round.tx, dl assert.strictEqual(shallow.accepted, false); assert.ok(/a path of \d+ levels; the wallet accepts 1 at most/.test(shallow.reason), shallow.reason); assert.throws(() => verifier({ minReserveAtoms: 0, minReserveFeeRate: { floorPerKvb: 1, multiple: 1 } }), /not both/); +// A field the policy does not name is refused, not ignored: a misspelt bound +// would otherwise take its default without a word. +assert.throws(() => verifier({ maxExitDelaySecond: 172800 }), /unknown field `maxExitDelaySecond`/); +assert.throws(() => verifier({ minReserveFeeRate: { floorPerKvb: 1, multiple: 1, mutliple: 4 } }), /unknown field `mutliple`/); // 2. The refusal vectors, by kind, from both the reader and the verifier. const any = v.batches[1];