feat(signing): canonical signing-bytes + ApprovedTxHash core (attested-signing PR2/10) by zmanian · Pull Request #3961 · nearai/ironclaw

zmanian · 2026-05-24T00:44:12Z

Summary

PR2 of the 10-PR attested-signing stack. Adds the ironclaw_attestation crate — the value-binding core that turns a server-decoded transaction into the binding ApprovedTxHash.

Stacks on #3960 (attested-signing-01-provider-trait), which defined the opaque ApprovedTxHash newtype and the SigningProvider trait. This PR targets that branch so the diff is clean; it will be rebased onto reborn-integration once PR1 merges.

Plan: docs/plans/2026-05-23-attested-signing-substrate.md.

What it delivers

decoded_tx.rs — chain-tagged, chain-SDK-free DecodedTransaction (Evm / Solana / Near) over plain serde types (Vec<u8>, String, u64, typed newtypes). Carries the signing-relevant fields from the plan's threat matrix: EVM (chain_id, nonce, tx_type, to, value, data, gas/fee caps, access_list, blob fields); Solana (ALT-resolved account keys, recent_blockhash, instructions, compute budget); NEAR (signer_id, receiver_id, access-key nonce, block_hash, actions, deposit, gas). RenderingSchemaVersion newtype.
fields.rs — the single shared field projection both the renderer and the canonical encoder consume.
rendered.rs — render(&DecodedTransaction, schema) derives the human view from that projection (no silent fields).
canonical.rs — canonical_signing_bytes(): deterministic, domain-separated, length-prefixed encoding.
approved_tx_hash.rs — compute_approved_tx_hash(): domain-separated SHA-256 over render ∥ canonical bytes ∥ signer/account ∥ chain/network ∥ tx-type ∥ schema-version. Returns the ApprovedTxHash from ironclaw_signing_provider.

Dependencies (conservative)

Depends ONLY on ironclaw_signing_provider, serde, thiserror, sha2. No chain SDK, no secrets, no webauthn (PR4/PR6). sha2 = "0.10" reuses the already-vendored 0.10.9 — no new transitive cost. The architecture boundary test is extended to assert this crate carries no chain/secrets/webauthn dependency.

Deviation from plan wording: the plan says "domain-separated CBOR". Per the conservative-deps rule I used a ~10-line hand-rolled length-prefixed encoder instead of adding ciborium. The binding property only needs an injective, domain-separated, deterministic encoding; length-prefixing every component (u32_be(len) ∥ bytes) gives exactly that. No new dependency proposed.

Anti-field-smuggling property (what the tests guarantee)

The renderer and the canonical encoder both derive from fields::project, so the approved view and the signed bytes cannot diverge ("approve view A, sign bytes B" is structurally impossible). Tests prove:

Anti-smuggling: changing ANY component (render, canonical bytes, signer/account, chain/network, tx-type, schema-version) changes the ApprovedTxHash; mutating any individual EVM field changes both the canonical bytes and the hash.
Determinism: identical DecodedTransaction ⇒ identical canonical bytes ⇒ identical hash, across repeated calls and across a serde round-trip.
Render coverage: per chain variant, render() surfaces every signing-relevant field the canonical encoder consumes.
Cross-chain separation: EVM and Solana txs with coincidentally-similar bytes hash differently; same tx on different EVM chain ids hashes differently.

Verification

cargo fmt --all — clean
cargo clippy -p ironclaw_attestation -p ironclaw_architecture --benches --tests --examples --all-features -- -D warnings — zero warnings
cargo test -p ironclaw_attestation — 13 passed
cargo test -p ironclaw_architecture --test attested_signing_boundaries — 2 passed (incl. the new attestation-purity assertion)

🤖 Generated with Claude Code

gemini-code-assist

Code Review

This pull request introduces the ironclaw_attestation crate, a core component for the attested-signing substrate that provides SDK-free transaction decoding, human-readable rendering, and canonical signing-bytes encoding. It features a shared field projection to ensure the human-approved view and signed bytes never diverge. Feedback focuses on improving the robustness of binary encodings by using u64 for length prefixes to avoid potential truncation or panics during usize conversion. Additionally, the reviewer noted a semantic inconsistency in the EVM signer_account implementation, which incorrectly returns the recipient address instead of the signer identity.

gemini-code-assist · 2026-05-24T00:47:28Z

+fn update_lp(hasher: &mut Sha256, bytes: &[u8]) {
+    hasher.update((bytes.len() as u32).to_be_bytes());
+    hasher.update(bytes);
+}


According to the general rules, binary encodings should prefer u64 for length prefixes to ensure infallible conversion from usize on all platforms and avoid potential truncation or collisions. This applies to update_lp, push_lp, and the field count encoding.

Suggested change

fn update_lp(hasher: &mut Sha256, bytes: &[u8]) {

hasher.update((bytes.len() as u32).to_be_bytes());

hasher.update(bytes);

}

fn update_lp(hasher: &mut Sha256, bytes: &[u8]) {

hasher.update((bytes.len() as u64).to_be_bytes());

hasher.update(bytes);

}

References

Prefer using u64 for length prefixes in binary encodings to ensure that conversions from usize are infallible on all supported platforms, avoiding panics and potential DoS vectors caused by .expect() on overflow.

Fixed in 5299324: update_lp now uses a u64 big-endian length prefix (usize->u64 is infallible on all supported platforms).

gemini-code-assist · 2026-05-24T00:47:28Z

+    push_lp(&mut out, rendered.chain.as_bytes());
+    push_lp(&mut out, rendered.chain_network.as_bytes());
+    push_lp(&mut out, rendered.tx_type.as_bytes());
+    out.extend_from_slice(&(rendered.fields.len() as u32).to_be_bytes());


Use u64 for the field count length prefix to maintain consistency with the general rule for binary encodings.

Suggested change

out.extend_from_slice(&(rendered.fields.len() as u32).to_be_bytes());

out.extend_from_slice(&(rendered.fields.len() as u64).to_be_bytes());

References

Prefer using u64 for length prefixes in binary encodings to ensure that conversions from usize are infallible on all supported platforms.

Fixed in 5299324: the render field-count prefix is now u64_be.

gemini-code-assist · 2026-05-24T00:47:28Z

+fn push_lp(out: &mut Vec<u8>, bytes: &[u8]) {
+    out.extend_from_slice(&(bytes.len() as u32).to_be_bytes());
+    out.extend_from_slice(bytes);
+}


Use u64 for the length prefix in push_lp to comply with the general rule for binary encodings.

Suggested change

fn push_lp(out: &mut Vec<u8>, bytes: &[u8]) {

out.extend_from_slice(&(bytes.len() as u32).to_be_bytes());

out.extend_from_slice(bytes);

}

fn push_lp(out: &mut Vec<u8>, bytes: &[u8]) {

out.extend_from_slice(&(bytes.len() as u64).to_be_bytes());

out.extend_from_slice(bytes);

}

References

Prefer using u64 for length prefixes in binary encodings to ensure that conversions from usize are infallible on all supported platforms.

Fixed in 5299324: push_lp in approved_tx_hash.rs now uses a u64_be length prefix.

gemini-code-assist · 2026-05-24T00:47:28Z

+//! u32_be(field_count)
+//! for each field, in canonical order:
+//!     lp(field.tag)
+//!     lp(field.canonical_bytes)
+//! ```
+//!
+//! where `lp(x)` = `u32_be(x.len()) ∥ x`. Length-prefixing every component makes


Update the documentation to reflect the change from u32 to u64 for length prefixes and field counts. Additionally, specify that lengths are in bytes to comply with repository standards for str::len() usage.

Suggested change

//! u32_be(field_count)

//! for each field, in canonical order:

//! lp(field.tag)

//! lp(field.canonical_bytes)

//! ```

//!

//! where `lp(x)` = `u32_be(x.len()) ∥ x`. Length-prefixing every component makes

//! u64_be(field_count)

//! for each field, in canonical order:

//! lp(field.tag)

//! lp(field.canonical_bytes)

//!

//!

//! where lp(x) = u64_be(x.len()) ∥ x. Lengths are in bytes. Length-prefixing every component makes

References

Prefer using u64 for length prefixes in binary encodings to ensure that conversions from usize are infallible on all supported platforms.

Specify bytes in documentation for string length limits if the implementation uses str::len() to avoid the performance overhead of chars().count().

Fixed in 5299324: canonical.rs module docs updated to u64_be length prefixes and field count, and now note lengths are in bytes.

gemini-code-assist · 2026-05-24T00:47:29Z

+fn push_lp(out: &mut Vec<u8>, bytes: &[u8]) {
+    out.extend_from_slice(&(bytes.len() as u32).to_be_bytes());
+    out.extend_from_slice(bytes);
+}


Use u64 for the length prefix in push_lp to comply with the general rule for binary encodings.

Suggested change

fn push_lp(out: &mut Vec<u8>, bytes: &[u8]) {

out.extend_from_slice(&(bytes.len() as u32).to_be_bytes());

out.extend_from_slice(bytes);

}

fn push_lp(out: &mut Vec<u8>, bytes: &[u8]) {

out.extend_from_slice(&(bytes.len() as u64).to_be_bytes());

out.extend_from_slice(bytes);

}

References

Prefer using u64 for length prefixes in binary encodings to ensure that conversions from usize are infallible on all supported platforms.

Fixed in 5299324: push_lp in canonical.rs now uses a u64_be length prefix.

gemini-code-assist · 2026-05-24T00:47:29Z

+    push_lp(&mut out, tx.chain_network().as_bytes());
+    push_lp(&mut out, tx.tx_type_label().as_bytes());
+    push_lp(&mut out, &schema_version.get().to_be_bytes());
+    out.extend_from_slice(&(fields.len() as u32).to_be_bytes());


Use u64 for the field count length prefix to maintain consistency with the general rule for binary encodings.

Suggested change

out.extend_from_slice(&(fields.len() as u32).to_be_bytes());

out.extend_from_slice(&(fields.len() as u64).to_be_bytes());

References

Prefer using u64 for length prefixes in binary encodings to ensure that conversions from usize are infallible on all supported platforms.

Fixed in 5299324: the canonical field-count prefix is now u64_be. The binding test parser was updated to match.

gemini-code-assist · 2026-05-24T00:47:29Z

+            DecodedTransaction::Evm(tx) => match tx.to {
+                // The EVM `from` is recovered from the signature at sign time
+                // (PR6, threat #5); the decode model has no `from`. The
+                // signer/account bound into the hash is supplied by the caller
+                // context, so here we surface the recipient for the render and
+                // leave signer binding to the explicit hash argument.
+                Some(addr) => format!("0x{}", hex_lower(&addr.0)),
+                None => "contract-creation".to_string(),
+            },


The signer_account() implementation for EVM returns the recipient (to) address, which is semantically incorrect for a method named signer_account. Furthermore, the recipient is already bound via the fields projection in fields.rs, making this redundant. The comment also incorrectly states that this surfaces the recipient for the render, but render() does not use this method. Consider returning an empty string for EVM to indicate the signer is not present in the transaction body, and rely on the caller to provide the actual signer identity from the context.

DecodedTransaction::Evm(_) => { // The EVM from is recovered from the signature at sign time // (PR6, threat #5); the decode model has no from. The // signer/account bound into the hash is supplied by the caller // context. We return an empty string here as the recipient is // already bound via the fields projection. String::new() },

Not applicable to the current code. There is no DecodedTransaction::signer_account() method, and nothing in the crate derives the signer from tx.to. The signer/account bound into the hash is the explicit, caller-supplied signer_account: &str parameter of approved_tx_hash_for (sourced from SigningContext.key_or_account_id), never recovered from the tx body — see the module docs in approved_tx_hash.rs. This appears to reference a stale revision; commit 2e65c2f already removed any tx-derived signer in favor of explicit binding.

Not applicable to the merged code — there is no signer_account() method and nothing derives the signer from tx.to. The signer bound into the hash is the explicit caller-supplied signer_account argument of approved_tx_hash_for, never recovered from the tx body (see approved_tx_hash.rs module docs). Stale-diff; explicit binding landed in 2e65c2f.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bf8797c902

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-24T00:48:08Z

+                Some(addr) => format!("0x{}", hex_lower(&addr.0)),
+                None => "contract-creation".to_string(),


Bind EVM signer to hash, not recipient address

DecodedTransaction::signer_account() is documented as the signer/account identity used in hash binding, but the EVM branch returns tx.to (or "contract-creation") instead of the signing account. If callers use this helper when computing ApprovedTxHash, two different EVM signers approving the same transaction payload to the same recipient can produce the same signer component, breaking the stated signer-binding invariant.

Useful? React with 👍 / 👎.

Not a bug in the current code. There is no DecodedTransaction::signer_account() helper and ApprovedTxHash does not bind any tx-derived signer. approved_tx_hash_for takes an explicit signer_account: &str (the trusted SigningContext.key_or_account_id) and folds it into the digest via update_lp, so two distinct EVM signers approving the same payload to the same recipient produce different hashes — exactly the signer-binding invariant you describe. The recipient is bound separately through the fields projection. This looks like it was generated against a stale diff; explicit-signer binding landed in 2e65c2f.

Confirmed not applicable to the merged code. There is no DecodedTransaction::signer_account() helper and ApprovedTxHash binds no tx-derived signer: approved_tx_hash_for takes an explicit signer_account: &str (the trusted SigningContext.key_or_account_id) folded in via update_lp, so two distinct EVM signers approving the same payload/recipient produce different hashes. The recipient is bound separately through the fields projection. Explicit-signer binding landed in 2e65c2f; this references a stale revision.

henrypark133

Code Review (multi-agent)

Intent: Adds the ironclaw_attestation crate to turn server-decoded transactions into canonical signing-bytes and ApprovedTxHash values under an anti-field-smuggling security property.

Stats: 11 findings across 5 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability. Reviewers failed: none. Body-only: 0

🛡️ Security

Critical Solana compact-u16 length truncation allows field smuggling and instruction injection (crates/ironclaw_attestation/src/wire.rs:45-50, confidence 100)
When push_compact_bytes is called with a byte slice larger than u16::MAX, casting bytes.len() as u16 silently truncates, allowing instruction injection.
High NEAR borsh byte-length truncation on 64-bit platforms (crates/ironclaw_attestation/src/wire.rs:105-110, confidence 100)
Casting bytes.len() (usize) as u32 silently truncates on 64-bit platforms if the byte slice length exceeds u32::MAX.
High NEAR unvalidated deposit/stake byte-length causes approve-vs-sign divergence (crates/ironclaw_attestation/src/wire.rs:117-136, confidence 100)
If the input byte slice has a length greater than 16 bytes, the excessive high bytes are silently discarded via wrapping shifts.

🐛 Bugs

High Missing actions serialization in NEAR Delegate action Borsh encoder (crates/ironclaw_attestation/src/wire.rs:220-233, confidence 100)
Because this field is completely omitted from the hand-rolled Borsh encoder in push_near_action, the resulting binary payload is malformed.

⚡ Performance

High Duplicate serialization of Solana message bytes in transaction projection (crates/ironclaw_attestation/src/fields.rs:232-237, confidence 98)
In the Solana variant of project, solana_message_bytes(sol) is called twice sequentially.
High Duplicate serialization of NEAR transaction bytes in transaction projection (crates/ironclaw_attestation/src/fields.rs:259-264, confidence 98)
In the NEAR variant of project, near_transaction_bytes(near) is called twice sequentially.
High Unbounded allocation/formatting of large transaction payload bytes (crates/ironclaw_attestation/src/fields.rs:69-76, confidence 90)
For large byte payloads, Field::bytes clones the entire payload and formats the whole slice as a lowercase hex string.
Medium Redundant transaction field projection in approved_tx_hash_for (crates/ironclaw_attestation/src/approved_tx_hash.rs:78-93, confidence 95)
The safe public API approved_tx_hash_for calls render and canonical_signing_bytes sequentially, executing the expensive project(tx) function independently.

🧪 Tests

Medium u128_from_be_minimal is not tested with oversized or malformed inputs (crates/ironclaw_attestation/src/wire.rs:130-136, confidence 100)
The function u128_from_be_minimal has no unit test exercising oversized inputs larger than 16 bytes.
Medium Near transaction type label is not tested with empty or multiple actions (crates/ironclaw_attestation/src/decoded_tx.rs:378-381, confidence 100)
The tx_type_label() method formatting of NEAR actions has no tests for empty or multiple actions.
Medium AddKey permission with empty method_names is not tested (crates/ironclaw_attestation/src/fields.rs:362-373, confidence 100)
There are no tests asserting the behavior of an empty method_names list.

henrypark133 · 2026-05-25T17:58:39Z

@@ -0,0 +1,285 @@
+//! Hand-rolled, chain-SDK-free serialization of the EXACT signed bytes for
+//! Solana versioned messages and NEAR transactions.


Critical — Solana compact-u16 length truncation allows field smuggling and instruction injection.

In release builds, debug_assert! is compiled out. When push_compact_bytes is called with a byte slice larger than u16::MAX, casting bytes.len() as u16 silently truncates. The compact-u16 length prefix will be written as a small value, but the entire massive byte slice is appended to the stream. This corrupts the serialized message layout, allowing an attacker to smuggle extra instructions or accounts that downstream parsers/verifiers will interpret as separate fields.

Fix: Validate that the byte slice length does not exceed u16::MAX and return a Result::Err instead of using debug_assert and silent truncation.

Fixed in follow-up PR #4067 (commit e5b6d21). push_compact_bytes no longer casts len as u16 behind a debug_assert; every Solana compact-u16 length now goes through checked_short_vec_len and returns AttestationError::SolanaShortVecOverflow for slices > u16::MAX. No truncation, so trailing bytes can never be reinterpreted as extra instructions/accounts. The encoders + project/render/canonical_signing_bytes/approved_tx_hash_for are now fallible. Covered by wire unit tests (overflow + exact-max boundary) and a binding integration test driving the safe public API.

henrypark133 · 2026-05-25T17:58:39Z

@@ -0,0 +1,285 @@
+//! Hand-rolled, chain-SDK-free serialization of the EXACT signed bytes for
+//! Solana versioned messages and NEAR transactions.


High — NEAR borsh byte-length truncation on 64-bit platforms.

In push_borsh_bytes, casting bytes.len() (usize) as u32 silently truncates on 64-bit platforms if the byte slice length exceeds u32::MAX. This writes a corrupted (truncated) length prefix followed by the entire large byte slice, resulting in serialization desynchronization and potential field smuggling or parsing exploits downstream.

Fix: Add a boundary check to ensure bytes.len() <= u32::MAX as usize and return an error on overflow.

Fixed in #4067 (e5b6d21). push_borsh_bytes now routes through borsh_len_le, which returns AttestationError::NearBorshLengthOverflow for lengths > u32::MAX instead of truncating on 64-bit platforms. Boundary covered by the borsh_bytes_rejects_overlong_length unit test.

henrypark133 · 2026-05-25T17:58:39Z

@@ -0,0 +1,285 @@
+//! Hand-rolled, chain-SDK-free serialization of the EXACT signed bytes for
+//! Solana versioned messages and NEAR transactions.


High — NEAR unvalidated deposit/stake byte-length causes approve-vs-sign divergence.

In push_borsh_u128_be, the be_minimal byte slice is parsed into a u128 by shifting left in u128_from_be_minimal. If the input byte slice has a length greater than 16 bytes, the excessive high bytes are silently discarded via wrapping shifts. Since there is no validation of the byte slice length during deserialization or serialization, the user is shown and approves the original untruncated large value in the render, but the signed wire bytes represent a truncated/wrapped (and potentially 0) value, creating a critical approve-vs-sign mismatch.

Fix: Validate that the length of be_minimal does not exceed 16 bytes during deserialization or before parsing, and return an error on overflow.

Fixed in #4067 (e5b6d21). u128_from_be_minimal now rejects inputs > 16 bytes with AttestationError::NearU128Overflow rather than wrapping/discarding high bytes via wrapping_shl. <=16-byte inputs (including leading-zero padding) still parse, so the canonical wire u128 always equals the human-rendered amount — no approve-vs-sign divergence. Covered by u128_be_minimal_oversized_bytes and the near_oversized_deposit_is_rejected integration test (also requested as finding #3299413230).

henrypark133 · 2026-05-25T17:58:39Z

@@ -0,0 +1,285 @@
+//! Hand-rolled, chain-SDK-free serialization of the EXACT signed bytes for
+//! Solana versioned messages and NEAR transactions.


High — Missing actions serialization in NEAR Delegate action Borsh encoder.

The NEAR DelegateAction wire format (NEP-366) requires a list of delegated actions between the receiver_id and the nonce fields. Because this field is completely omitted from the hand-rolled Borsh encoder in push_near_action, the resulting binary payload is malformed. This causes deserializers to misinterpret the nonce as the actions vector length, leading to corrupted parses, potential panic, or signature verification failure in production.

Fix: Serialize a zero-length actions vector (as an empty list [0, 0, 0, 0] representing 0u32 in Borsh) or update the enum and serializer to properly support and encode delegated actions.

Fixed in #4067 (e5b6d21). The Delegate encoder now serializes an explicit empty Vec<Action> (borsh 0u32) in the NEP-366 position between receiver_id and nonce, so a deserializer no longer reads the 8-byte nonce as the actions-vector length. The decoded model carries no inner delegated actions, hence the empty vector. Verified by near_delegate_action_serializes_empty_inner_actions_vector, which asserts the exact wire fragment.

henrypark133 · 2026-05-25T17:58:39Z

+//! ## Safe vs low-level API
+//!
+//! Callers should use [`approved_tx_hash_for`], which derives BOTH the render
+//! and the canonical signing bytes from the *same* decoded transaction and the


Medium — Redundant transaction field projection in approved_tx_hash_for.

The safe public API approved_tx_hash_for calls render and canonical_signing_bytes sequentially, both of which execute the expensive project(tx) function independently. This causes redundant string formatting, vector cloning, and hex encoding on every hash computation.

Fix: Expose a way to pass pre-projected fields or refactor approved_tx_hash_for to call project once.

Fixed in #4067 (e5b6d21). fields::project now calls solana_message_bytes(sol) once, storing the result and reusing it for both the human value and canonical_bytes.

(see threaded reply above)

henrypark133 · 2026-05-25T17:58:39Z

+                project_near_action(&mut fields, action);
+            }
+            // Bind the EXACT borsh-signed transaction bytes.
+            fields.push(Field {


High — Duplicate serialization of NEAR transaction bytes in transaction projection.

In the NEAR variant of project, near_transaction_bytes(near) is called twice sequentially: once to format the hex value and once to populate the canonical bytes. Since project is called twice per hash calculation, the entire transaction (including all actions/payloads) is serialized four times.

Fix: Call near_transaction_bytes(near) once, store the result, and use it for both fields.

Fixed in #4067 (e5b6d21). near_transaction_bytes(near) is now called once in project and reused for both the rendered value and canonical_bytes.

henrypark133 · 2026-05-25T17:58:39Z

+        }
+    }
+
+    fn bytes(tag: &'static str, label: &'static str, bytes: &[u8]) -> Self {


High — Unbounded allocation/formatting of large transaction payload bytes.

For large byte payloads (such as WASM contract code in NEAR DeployContract), Field::bytes clones the entire payload and formats the whole slice as a lowercase hex string. This creates multiple megabytes of redundant heap allocations on hot/regular signature validation paths.

Fix: Limit/truncate the human-readable hex display of very large payloads (e.g., over 1KB) while keeping the full bytes in canonical_bytes.

Deferring in #4067; not a correctness/security issue. The full payload must remain in canonical_bytes (it is the signed commitment). Truncating only the human hex display risks a display/canonical divergence and adds complexity; on the signing path this runs once per approval, not in a tight loop. Can revisit with an explicit display cap if profiling shows it matters.

henrypark133 · 2026-05-25T17:58:39Z

+/// Append a yoctoNEAR `u128` carried as big-endian minimal bytes, re-encoded as
+/// borsh `u128` (little-endian, 16 bytes).
+fn push_borsh_u128_be(out: &mut Vec<u8>, be_minimal: &[u8]) {
+    let value = u128_from_be_minimal(be_minimal);


Medium — u128_from_be_minimal is not tested with oversized or malformed inputs.

The function u128_from_be_minimal parses big-endian minimal byte slices. It contains recovery logic for malformed or oversized inputs (bytes length > 16) but has no unit test exercising this case.

Fix: tests::wire::u128_be_minimal_oversized_bytes covering oversized inputs larger than 16 bytes

Added in #4067 (e5b6d21): tests::wire::u128_be_minimal_oversized_bytes exercises the >16-byte path (now a hard error, see #3299413217), plus the near_oversized_deposit_is_rejected integration test through the public API.

henrypark133 · 2026-05-25T17:58:39Z

+//!
+//! ## Full-fidelity message modeling
+//!
+//! Solana and NEAR are modeled at *full signing fidelity*: the Solana variant


Medium — Near transaction type label is not tested with empty or multiple actions.

The tx_type_label() method formatting of NEAR actions (kinds joined by commas) is never tested for edge cases like empty actions (resulting in "near-actions[]") or multiple actions (resulting in e.g. "near-actions[Transfer,CreateAccount]").

Fix: tests::binding::near_tx_type_label_edge_cases covering empty and multiple actions

Acknowledged. Not added in #4067 (scope limited to the wire-encoding security findings). tx_type_label() for empty/multiple NEAR actions is a label-formatting edge case with no security impact; happy to add the suggested near_tx_type_label_edge_cases test in a separate cleanup.

henrypark133 · 2026-05-25T17:58:39Z

+                public_key,
+            ));
+        }
+        NearAction::AddKey {


Medium — AddKey permission with empty method_names is not tested.

The project_near_action projection logic iterates over method_names to push Field::text / Field::num elements. If method_names is empty (signifying permission to call any method on-chain), this code path is bypassed. There are no tests asserting the behavior of an empty method_names list.

Fix: tests::binding::near_addkey_empty_method_names covering AddKey permission with empty method_names

Acknowledged. Not added in #4067 (scope limited to wire-encoding security findings). The empty-method_names AddKey path is already bound (the FunctionCall permission + receiver are emitted regardless); a dedicated near_addkey_empty_method_names assertion is reasonable as a follow-up test-coverage PR.

Widen canonical/approved-tx length prefixes and field counts from u32 to u64 in the hand-rolled length-prefixed encoders (canonical.rs and approved_tx_hash.rs). usize->u64 is infallible on all supported platforms, removing any theoretical truncation/collision on the canonical signing pre-image. Updated module docs and the binding-test parser to match. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

zmanian · 2026-05-26T01:23:04Z

Review follow-up (henrypark133 CHANGES_REQUESTED)

This PR was already merged into the stack's attested-signing-01-provider-trait branch, so the wire-encoding findings are addressed in follow-up PR #4067 (commit e5b6d2142) on top of that branch.

Disposition

Finding	Severity	Disposition
compact-u16 length truncation (field/instruction smuggling)	Critical	FIXED #4067 — `checked_short_vec_len` → `SolanaShortVecOverflow`, no `as u16` truncation
NEAR borsh u32 truncation on 64-bit	High	FIXED #4067 — `borsh_len_le` → `NearBorshLengthOverflow`
NEAR u128 wrapping discard (>16 bytes; approve-vs-sign divergence)	High	FIXED #4067 — `u128_from_be_minimal` rejects >16 bytes → `NearU128Overflow`
NEAR `Delegate` missing `actions` vector	High	FIXED #4067 — explicit empty `Vec<Action>` (`0u32`) in NEP-366 position
Duplicate Solana wire serialization	High	FIXED #4067 — serialize once, reuse
Duplicate NEAR wire serialization	High	FIXED #4067 — serialize once, reuse
`u128_from_be_minimal` oversized test	Medium	ADDED #4067 — unit + integration test
Redundant projection in `approved_tx_hash_for`	Medium	DECLINED — deriving render+canonical from the same `tx` is the safety property that prevents approve-A/sign-B; per-call double-serialization fixed
Large hex display allocation	High (perf)	DEFERRED — full bytes must stay in canonical commitment; truncating display risks divergence; not a hot path
`tx_type_label` empty/multi-action test	Medium	ACK — label-only, no security impact; follow-up test PR
AddKey empty `method_names` test	Medium	ACK — follow-up test PR
EVM signer bound to recipient (×2)	P1/Medium	NOT APPLICABLE — no `signer_account()` method exists; explicit caller-supplied signer landed in `2e65c2f31` (stale diff)

Security note (WYSIWYS)

The encoders are now fail-closed: an input that cannot be reproduced byte-for-byte as the wallet/HSM-signed payload is rejected (AttestationError) rather than truncated/wrapped. project/render/canonical_signing_bytes/approved_tx_hash_for are now fallible. Crate stays chain-SDK-free / openssl-free; architecture boundary test passes; cargo fmt --check + clippy --all-features (zero warnings) + test --all-features all green.

…es (nearai#3961 follow-up) Address henrypark133 review on nearai#3961 (merged): the hand-rolled Solana/NEAR wire encoders in ironclaw_attestation silently truncated length prefixes, which breaks the what-you-see-is-what-you-sign (WYSIWYS) binding by letting an attacker desync a length from its payload. Critical (compact-u16): push_compact_bytes cast bytes.len() as u16 behind a debug_assert (compiled out in release). A slice > u16::MAX truncated the shortvec prefix while appending the full slice, so trailing bytes could be reinterpreted as extra instructions/accounts (field/instruction smuggling). Now every Solana shortvec length goes through checked_short_vec_len and returns AttestationError::SolanaShortVecOverflow instead of truncating. High (NEAR borsh u32): push_borsh_bytes cast len as u32, truncating on 64-bit platforms for slices > u32::MAX. Now routed through borsh_len_le, returning AttestationError::NearBorshLengthOverflow. High (NEAR u128): u128_from_be_minimal used wrapping shifts, silently discarding high bytes for inputs > 16 bytes — the rendered (full) value would diverge from the signed (wrapped) value. Now rejects >16-byte inputs with AttestationError::NearU128Overflow; <=16 bytes (incl. leading-zero padding) still parse. High (NEAR Delegate): the Delegate action omitted the NEP-366 Vec<Action> field, so a deserializer would read nonce as the actions-vector length. Now serializes an explicit empty actions vector (borsh 0u32) in the correct position between receiver_id and nonce. The encoders, fields::project, render, canonical_signing_bytes, and approved_tx_hash_for are now fallible (Result<_, AttestationError>). Solana and NEAR wire bytes are also serialized ONCE and reused for both the human value and the canonical commitment (was twice each), addressing the duplicate-serialization findings. Adds wire unit tests (overflow boundaries) and binding integration tests that drive the safe public API for each rejection path plus the Delegate layout. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

github-actions Bot added scope: dependencies Dependency updates size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels May 24, 2026

gemini-code-assist Bot reviewed May 24, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed May 24, 2026

View reviewed changes

zmanian requested review from henrypark133 and serrrfirat May 24, 2026 23:28

zmanian mentioned this pull request May 24, 2026

feat(signing): SigningProvider trait crate + plan doc (attested-signing PR1/10) #3960

Closed

henrypark133 requested changes May 25, 2026

View reviewed changes

zmanian merged commit 63ce687 into attested-signing-01-provider-trait May 26, 2026
15 checks passed

zmanian deleted the attested-signing-02-canonical-hash branch May 26, 2026 01:13

zmanian mentioned this pull request May 26, 2026

fix(signing): fail-closed wire encoding for attestation canonical bytes (#3961 follow-up) #4067

Closed

Haderach-Ram mentioned this pull request May 26, 2026

Ecosystem Digest — 2026-05-26 Haderach-Ram/openclaw-radar#19

Open

github-actions Bot mentioned this pull request May 26, 2026

🦞 OpenClaw 生态日报 2026-05-26 ivanweng2077/big_model_radar#92

Open

	out.extend_from_slice(&(rendered.fields.len() as u32).to_be_bytes());
	out.extend_from_slice(&(rendered.fields.len() as u64).to_be_bytes());

	out.extend_from_slice(&(fields.len() as u32).to_be_bytes());
	out.extend_from_slice(&(fields.len() as u64).to_be_bytes());

		Some(addr) => format!("0x{}", hex_lower(&addr.0)),
		None => "contract-creation".to_string(),

		@@ -0,0 +1,285 @@
		//! Hand-rolled, chain-SDK-free serialization of the EXACT signed bytes for
		//! Solana versioned messages and NEAR transactions.

Conversation

zmanian commented May 24, 2026

Summary

What it delivers

Dependencies (conservative)

Anti-field-smuggling property (what the tests guarantee)

Verification

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

henrypark133 left a comment

Choose a reason for hiding this comment

Code Review (multi-agent)

🛡️ Security

🐛 Bugs

⚡ Performance

🧪 Tests

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!