chore: strip audit-ticket prefixes from code comments
Drops the ~60 ticket-prefix comments (CRIT-N, HIGH-N, MED-N, LOW-N, L-N, M-N, AUDIT-N, PLUTUS-N, "audit fix (date):", "Phase N" labels, "Adversarial-review fix:") that had accumulated in inline + doc comments over several audit cycles. Where the surrounding prose still carried useful WHY context it gets kept and tightened; where the ticket WAS the comment it gets dropped entirely. No logic, no renames, no behavior change. Audit history lives in commit messages and the audits/ tree where it belongs — eternal comments don't need to mirror it. Net 138 LOC shorter. 253 tests pass, no new clippy or fmt warnings.
This commit is contained in:
parent
4e4becd0bb
commit
b69d891a2b
12 changed files with 201 additions and 339 deletions
|
|
@ -44,6 +44,5 @@ rpassword = { workspace = true }
|
|||
zeroize = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
# CRIT-1/CRIT-2 audit fix tests (2026-05-12): need a real tx CBOR
|
||||
# fixture + temp-dir sandbox root.
|
||||
# Sandbox + cap-check tests need a temp-dir + tx CBOR fixture.
|
||||
tempfile = "3"
|
||||
|
|
|
|||
|
|
@ -31,10 +31,9 @@ use anyhow::{anyhow, Context, Result};
|
|||
use zeroize::Zeroizing;
|
||||
|
||||
/// Atomically create a file with `0o600` permissions and write the
|
||||
/// payload. Replaces the older `fs::write` + `chmod` two-step which
|
||||
/// had a TOCTOU window where the file existed with default umask
|
||||
/// perms (often `0o644`) before the chmod tightened it.
|
||||
/// (M-2 audit fix.)
|
||||
/// payload. Avoids the `fs::write` + `chmod` TOCTOU window where the
|
||||
/// file briefly exists at default umask (often `0o644`) before the
|
||||
/// chmod tightens it.
|
||||
#[cfg(unix)]
|
||||
fn write_owner_only(path: &Path, payload: &[u8]) -> Result<()> {
|
||||
use std::os::unix::fs::OpenOptionsExt;
|
||||
|
|
@ -55,9 +54,8 @@ fn write_owner_only(path: &Path, payload: &[u8]) -> Result<()> {
|
|||
}
|
||||
|
||||
/// Read `ALDABRA_PASSPHRASE` env into a `Zeroizing<String>` so the
|
||||
/// in-process copy gets wiped when dropped. The env block itself
|
||||
/// isn't zeroizable — that's a documented headless tradeoff. (M-3
|
||||
/// audit fix.)
|
||||
/// in-process copy gets wiped on drop. The env block itself isn't
|
||||
/// zeroizable — documented headless tradeoff.
|
||||
fn passphrase_from_env() -> Option<Zeroizing<String>> {
|
||||
std::env::var("ALDABRA_PASSPHRASE").ok().map(Zeroizing::new)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,20 +3,8 @@
|
|||
//! Speaks MCP over stdio. Any MCP client (e.g. Claude Code)
|
||||
//! launches this as a subprocess and gets a wallet's worth of tools.
|
||||
//!
|
||||
//! ## Phase 1 tools (target — server wiring lands in 1.7)
|
||||
//!
|
||||
//! - `wallet.address` — derived CIP-1852 base address
|
||||
//! - `wallet.balance` — ADA + native-asset balance via chain backend
|
||||
//! - `wallet.utxos` — list UTXOs at the wallet address
|
||||
//! - `wallet.network` — configured network selector
|
||||
//!
|
||||
//! ## Phase 2-4 tools
|
||||
//!
|
||||
//! See `ROADMAP.md` at the repo root.
|
||||
//!
|
||||
//! ## Logging
|
||||
//!
|
||||
//! Stderr only — stdout is the MCP transport, must stay clean.
|
||||
//! Logging: stderr only — stdout is the MCP transport and must stay
|
||||
//! clean.
|
||||
|
||||
mod bootstrap;
|
||||
mod config;
|
||||
|
|
@ -60,17 +48,12 @@ async fn run() -> Result<()> {
|
|||
"aldabra starting"
|
||||
);
|
||||
|
||||
// CRIT-2 audit fix (2026-05-12): make sure the sandbox root exists
|
||||
// and is daemon-only readable. Tools use canonicalize() against
|
||||
// this dir to validate `*_path` args, which requires the dir to
|
||||
// be a real directory on disk. Idempotent: create_dir_all is a
|
||||
// no-op when the dir already exists.
|
||||
//
|
||||
// Adversarial-review fix (2026-05-12): chmod is `?`'d not swallowed.
|
||||
// If the filesystem refuses chmod (noexec, selinux, broken mount),
|
||||
// we'd otherwise silently fall back to the umask default (commonly
|
||||
// 0o755) — making the security comment "daemon-only readable" a
|
||||
// lie. Fail loudly instead so the operator sees + investigates.
|
||||
// Make sure the sandbox root exists and is daemon-only readable.
|
||||
// Tools canonicalize() against this dir to validate `*_path`
|
||||
// args, which requires the dir to be a real directory on disk.
|
||||
// Chmod is `?`'d not swallowed — if the filesystem refuses it
|
||||
// (noexec, selinux, broken mount), fail loudly instead of
|
||||
// silently falling back to umask 0o755.
|
||||
if !cfg.safe_reads_root.exists() {
|
||||
std::fs::create_dir_all(&cfg.safe_reads_root).map_err(|e| {
|
||||
anyhow::anyhow!(
|
||||
|
|
@ -117,8 +100,8 @@ async fn run() -> Result<()> {
|
|||
let xprv_path = bootstrap::root_xprv_path(&cfg.data_dir);
|
||||
let any_key_exists = mnemonic_path.exists() || xprv_path.exists();
|
||||
|
||||
// L-2 audit fix: scope `root` to a block so its XPrv drops + wipes
|
||||
// as soon as we've extracted the keys we need.
|
||||
// Scope `root` to a block so its XPrv drops + wipes as soon as
|
||||
// we've extracted the keys we need.
|
||||
let (payment_key, stake_key, address) = {
|
||||
let root = if bootstrap_new {
|
||||
bootstrap::generate_and_save_root_key(&cfg.data_dir)?
|
||||
|
|
|
|||
|
|
@ -5,23 +5,9 @@
|
|||
//! tool names against `[a-zA-Z0-9_-]{1,64}` and silently drops names
|
||||
//! with dots, causing the daemon to run without advertising any tools.
|
||||
//!
|
||||
//! ## Phase 1 — read path
|
||||
//!
|
||||
//! - `wallet_address` — bech32 base address
|
||||
//! - `wallet_network` — mainnet | preview | preprod
|
||||
//! - `wallet_balance` — JSON `{lovelace, assets}`
|
||||
//! - `wallet_utxos` — JSON list of UTXOs
|
||||
//!
|
||||
//! ## Phase 2 — send path
|
||||
//!
|
||||
//! - `wallet_send` — build + sign + submit ADA payment, with hard
|
||||
//! cap guard (`max_send_lovelace`)
|
||||
//! - `wallet_tx_status` — poll a submitted tx hash
|
||||
//!
|
||||
//! Returns:
|
||||
//! - `String` results pass through `IntoContents` directly.
|
||||
//! - `Result<String, String>` lets us surface chain / build errors
|
||||
//! as MCP tool-call errors instead of crashing the daemon.
|
||||
//! Result types: `String` passes through `IntoContents` directly;
|
||||
//! `Result<String, String>` surfaces chain / build errors as MCP
|
||||
//! tool-call errors instead of crashing the daemon.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
|
|
@ -78,29 +64,17 @@ use aldabra_dao::discovery::{
|
|||
};
|
||||
use aldabra_dao::reader::{DaoReader, KoiosDaoReader};
|
||||
|
||||
/// CRIT-2 audit fix (2026-05-12): reject any `*_path` arg whose
|
||||
/// canonical form does not live under the configured sandbox root.
|
||||
/// Without this guard, a prompt-injection that gets the LLM to call a
|
||||
/// tool with `reference_script_path: "/var/lib/aldabra/root-xprv.age"`
|
||||
/// would have the daemon happily read the encrypted key blob. Even
|
||||
/// though hex decoding will fail, the previous error message included
|
||||
/// the byte offset of the first non-hex character — a small but real
|
||||
/// information leak on the file's binary structure. This guard +
|
||||
/// content-stripped error messages close both surfaces.
|
||||
/// Reject any `*_path` arg whose canonical form does not live under
|
||||
/// the sandbox root. Without this, a prompt-injection could point the
|
||||
/// daemon at `$ALDABRA_DATA/root-xprv.age` and leak key bytes through
|
||||
/// decode-error messages. Both root and arg are canonicalized
|
||||
/// (symlinks resolved, must exist); arg must `starts_with` root.
|
||||
///
|
||||
/// `safe_reads_root` and `user_path` are both canonicalized (resolves
|
||||
/// symlinks, requires the file to exist). The user path must
|
||||
/// `starts_with` the root; equality is allowed but rare in practice.
|
||||
///
|
||||
/// Adversarial-review fix (2026-05-12): also reject regular files
|
||||
/// whose link count > 1. `canonicalize` resolves symlinks but NOT
|
||||
/// hardlinks — a hardlink IS the file (same inode, multiple directory
|
||||
/// entries). Without this check, an attacker with daemon-uid write
|
||||
/// access could plant a hardlink inside the sandbox pointing at the
|
||||
/// inode of `$ALDABRA_DATA/root-xprv.age` and exfiltrate key bytes
|
||||
/// through the read path. No legitimate use case puts hardlinks in
|
||||
/// a script-CBOR sandbox; refusing them outright is conservative
|
||||
/// and easy.
|
||||
/// Hardlinks are refused too: `canonicalize` resolves symlinks but a
|
||||
/// hardlink IS the file. If an attacker with daemon-uid write access
|
||||
/// plants a hardlink to a secret inside the sandbox, the inode is
|
||||
/// inside-sandbox by every filesystem-level check. Nothing legitimate
|
||||
/// puts hardlinks in a script-CBOR dir, so `nlink > 1` → refuse.
|
||||
fn assert_inside_sandbox(safe_reads_root: &Path, user_path: &str) -> Result<PathBuf, String> {
|
||||
let user_canon = std::fs::canonicalize(user_path)
|
||||
.map_err(|e| format!("resolve path: {e} (path must exist + be readable)"))?;
|
||||
|
|
@ -117,12 +91,9 @@ fn assert_inside_sandbox(safe_reads_root: &Path, user_path: &str) -> Result<Path
|
|||
root_canon.display()
|
||||
));
|
||||
}
|
||||
// Hardlink rejection. `metadata` follows the post-canonicalize path
|
||||
// directly to the file (no symlink traversal needed since canonicalize
|
||||
// already resolved them). nlink > 1 means another directory entry
|
||||
// points at the same inode; we can't verify the other entry is also
|
||||
// inside the sandbox without an exhaustive filesystem scan, so we
|
||||
// reject conservatively.
|
||||
// nlink > 1 means another directory entry points at the same inode.
|
||||
// We can't verify the other entry is also inside the sandbox without
|
||||
// a full filesystem scan, so reject conservatively.
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
|
|
@ -139,24 +110,16 @@ fn assert_inside_sandbox(safe_reads_root: &Path, user_path: &str) -> Result<Path
|
|||
Ok(user_canon)
|
||||
}
|
||||
|
||||
/// Resolve a reference-script bytestring from EITHER an inline hex
|
||||
/// argument OR a file path inside the container. Caller passes both
|
||||
/// raw options; this fn enforces the "at most one" rule and reads
|
||||
/// the file when path is set.
|
||||
/// Resolve a reference-script bytestring from either an inline hex
|
||||
/// argument or a file path. At most one may be set.
|
||||
///
|
||||
/// The path-based variant exists because of an MCP transport bug:
|
||||
/// hex strings >~ 4500 chars get a 1-byte truncation + structural
|
||||
/// rearrangement somewhere between client and stdio reader. Reading
|
||||
/// from a file inside the container bypasses the JSON-RPC arg path
|
||||
/// entirely.
|
||||
/// The path variant exists because MCP truncates hex strings over
|
||||
/// ~4500 chars somewhere between client and stdio reader. Reading
|
||||
/// from a file bypasses the JSON-RPC arg path entirely.
|
||||
///
|
||||
/// CRIT-2 audit fix (2026-05-12): the path arg is sandboxed via
|
||||
/// [`assert_inside_sandbox`] against the daemon's configured
|
||||
/// `safe_reads_root` (default `$ALDABRA_DATA/scripts/`). Decode-error
|
||||
/// messages no longer include the file path or byte-offset context —
|
||||
/// only "could not decode as hex" — to avoid leaking structural
|
||||
/// information about non-hex files (e.g. encrypted key blobs that an
|
||||
/// attacker might point the tool at).
|
||||
/// Path reads are sandboxed by [`assert_inside_sandbox`]. Decode
|
||||
/// errors on path contents return a constant message — no byte-offset
|
||||
/// or file-content leak.
|
||||
fn resolve_ref_script_bytes(
|
||||
safe_reads_root: &Path,
|
||||
cbor_hex: Option<&str>,
|
||||
|
|
@ -180,10 +143,8 @@ fn resolve_ref_script_bytes(
|
|||
if cleaned.is_empty() {
|
||||
return Err("reference_script_path contained no hex characters".into());
|
||||
}
|
||||
// CRIT-2: do NOT include the hex_decode error's position-
|
||||
// offset detail or the file's contents in the user-visible
|
||||
// error. A failed decode is a yes/no signal; anything more
|
||||
// narrows the attacker's guesses about file structure.
|
||||
// Constant message — leaking byte offsets would narrow an
|
||||
// attacker's guesses about file structure.
|
||||
hex_decode(&cleaned)
|
||||
.map(Some)
|
||||
.map_err(|_| "reference_script_path contents are not valid hex".to_string())
|
||||
|
|
@ -192,19 +153,10 @@ fn resolve_ref_script_bytes(
|
|||
}
|
||||
}
|
||||
|
||||
/// Resolve the Plutus minting-policy CBOR from EITHER an inline
|
||||
/// hex argument OR a file path inside the container. Caller passes
|
||||
/// both raw options; this fn enforces the "exactly one" rule and
|
||||
/// reads the file when path is set.
|
||||
///
|
||||
/// Mirrors [`resolve_ref_script_bytes`] — same workaround for the
|
||||
/// MCP large-string transport bug where hex strings >~ 4500 chars
|
||||
/// get a 1-byte truncation between client and stdio reader,
|
||||
/// surfacing as "odd length" hex decode errors and blocking debug-
|
||||
/// build minting policies. Reading from a file inside the container
|
||||
/// bypasses the JSON-RPC arg path entirely.
|
||||
///
|
||||
/// CRIT-2 audit fix (2026-05-12): see [`resolve_ref_script_bytes`].
|
||||
/// Resolve a Plutus minting-policy CBOR from either inline hex or
|
||||
/// a file path. Exactly one must be set. Mirrors
|
||||
/// [`resolve_ref_script_bytes`] — same transport workaround, same
|
||||
/// sandbox + constant-message contract.
|
||||
fn resolve_policy_cbor_bytes(
|
||||
safe_reads_root: &Path,
|
||||
cbor_hex: Option<&str>,
|
||||
|
|
@ -325,10 +277,9 @@ struct WalletInner {
|
|||
/// quotas. None = public tier. Sourced from env only — never from
|
||||
/// disk; never logged.
|
||||
koios_bearer: Option<String>,
|
||||
/// CRIT-2 audit fix (2026-05-12): sandbox root for `*_path` tool
|
||||
/// args. Files outside this directory (canonical) are refused to
|
||||
/// keep prompt-injection from steering reads at the encrypted
|
||||
/// key material under `$ALDABRA_DATA/`.
|
||||
/// Sandbox root for `*_path` tool args. Files outside this dir
|
||||
/// (canonical) are refused, keeping prompt-injection from steering
|
||||
/// reads at the encrypted key material under `$ALDABRA_DATA/`.
|
||||
safe_reads_root: PathBuf,
|
||||
}
|
||||
|
||||
|
|
@ -386,10 +337,9 @@ impl WalletService {
|
|||
}
|
||||
|
||||
/// Reject if `lovelace` exceeds the wallet's hard cap unless
|
||||
/// `force=true`. Used by every tool that moves lovelace to a
|
||||
/// non-wallet destination — wallet_send, wallet_mint,
|
||||
/// wallet_mint_cip68_nft, wallet_script_spend.
|
||||
/// (HIGH-1 audit fix: previously only wallet_send had this guard.)
|
||||
/// `force=true`. Every tool that moves lovelace to a non-wallet
|
||||
/// destination calls this — `wallet_send`, `wallet_mint`,
|
||||
/// `wallet_mint_cip68_nft`, `wallet_script_spend`.
|
||||
fn enforce_value_cap(&self, lovelace: u64, force: bool) -> Result<(), String> {
|
||||
if lovelace > self.inner.max_send_lovelace && !force {
|
||||
return Err(format!(
|
||||
|
|
@ -400,23 +350,16 @@ impl WalletService {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// CRIT-1 audit fix (2026-05-12): scan a Conway-era tx CBOR and
|
||||
/// enforce the wallet's `max_send_lovelace` cap against the sum
|
||||
/// of lovelace going to any address that isn't this wallet's own.
|
||||
/// This is the chokepoint that defends `wallet_sign_partial` and
|
||||
/// `wallet_submit_signed_tx` against an unsigned-tx →
|
||||
/// sign-partial → submit drain via prompt injection. Each of
|
||||
/// those tools individually had `lovelace > 0` validation but no
|
||||
/// blast-radius limit.
|
||||
/// Chokepoint for `wallet_sign_partial` and `wallet_submit_signed_tx`:
|
||||
/// sum lovelace going to any non-self address and apply
|
||||
/// `enforce_value_cap`. Without this, a prompt-injection can walk
|
||||
/// the unsigned → sign → submit chain and drain past the cap —
|
||||
/// each step alone only checks `lovelace > 0`.
|
||||
///
|
||||
/// "Non-self" is computed by comparing the wallet's bech32 address
|
||||
/// (decoded to raw bytes via pallas-addresses, then hexed) against
|
||||
/// each output's `address_hex` in the tx body. Outputs whose
|
||||
/// address matches are treated as change/self-send and excluded.
|
||||
/// This is intentionally conservative on the same wallet's other
|
||||
/// accounts/indexes — they'll be flagged as non-self until/unless
|
||||
/// the wallet learns additional addresses to consider its own.
|
||||
/// Force `=true` overrides; mirror the `enforce_value_cap` contract.
|
||||
/// "Non-self" matches the wallet's primary bech32 address only.
|
||||
/// Other accounts/indexes of the same wallet are conservatively
|
||||
/// counted as non-self. `force=true` overrides, same contract as
|
||||
/// `enforce_value_cap`.
|
||||
fn enforce_cap_on_cbor(&self, cbor_bytes: &[u8], force: bool) -> Result<(), String> {
|
||||
let total = sum_non_self_lovelace(cbor_bytes, &self.inner.address)?;
|
||||
self.enforce_value_cap(total, force)
|
||||
|
|
@ -428,13 +371,10 @@ impl WalletService {
|
|||
/// function so the cap-check logic is unit-testable without standing
|
||||
/// up a full WalletService.
|
||||
///
|
||||
/// Adversarial-review fix (2026-05-12): `try_fold` + `checked_add`
|
||||
/// instead of `.sum::<u64>()`. The naive `.sum()` wraps on overflow
|
||||
/// in release builds — a prompt-injection could craft a CBOR with
|
||||
/// outputs summing past `u64::MAX`, wrap to a small value, and
|
||||
/// silently pass the cap check. Real-world unreachable (Cardano max
|
||||
/// supply is ~45B ADA = 4.5e16 lovelace, ~400x below `u64::MAX`) but
|
||||
/// trivial to harden.
|
||||
/// Uses `try_fold` + `checked_add` rather than `.sum::<u64>()` so a
|
||||
/// crafted CBOR that overflows can't silently wrap to a small value
|
||||
/// in release builds. Cardano max supply is ~400x below `u64::MAX`
|
||||
/// so the overflow path is unreachable in practice, but free to harden.
|
||||
fn sum_non_self_lovelace(cbor_bytes: &[u8], wallet_bech32: &str) -> Result<u64, String> {
|
||||
use pallas_addresses::Address;
|
||||
let wallet_addr =
|
||||
|
|
@ -587,10 +527,9 @@ pub struct UnsignedSendArgs {
|
|||
/// [`SendArgs::reference_script_kind`].
|
||||
#[serde(default)]
|
||||
pub reference_script_kind: Option<String>,
|
||||
/// CRIT-1 audit fix (2026-05-12): bypass the configured
|
||||
/// `max_send_lovelace` hard cap. Required to build an unsigned
|
||||
/// tx that would otherwise drain the wallet past the cap once
|
||||
/// signed + submitted. Mirrors `wallet_send.force`.
|
||||
/// Bypass the `max_send_lovelace` cap. Required when an unsigned
|
||||
/// tx would otherwise drain the wallet past the cap once signed
|
||||
/// and submitted. Mirrors `wallet_send.force`.
|
||||
#[serde(default)]
|
||||
pub force: bool,
|
||||
}
|
||||
|
|
@ -601,12 +540,10 @@ pub struct SubmitSignedArgs {
|
|||
/// cold-signer that consumed the unsigned CBOR returned by
|
||||
/// `wallet_send_unsigned`.
|
||||
pub signed_cbor_hex: String,
|
||||
/// CRIT-1 audit fix (2026-05-12): bypass the configured
|
||||
/// `max_send_lovelace` hard cap. The submit path scans the tx
|
||||
/// CBOR and sums lovelace going to any non-self address; pass
|
||||
/// `force=true` to push a tx that exceeds that cap (cold-signed
|
||||
/// multi-sig flows where a treasury moves real value
|
||||
/// intentionally).
|
||||
/// Bypass the `max_send_lovelace` cap. The submit path scans the
|
||||
/// tx CBOR and sums lovelace going to any non-self address; pass
|
||||
/// `force=true` for cold-signed multi-sig flows where a treasury
|
||||
/// moves real value intentionally.
|
||||
#[serde(default)]
|
||||
pub force: bool,
|
||||
}
|
||||
|
|
@ -679,10 +616,9 @@ pub struct PlutusMintUnsignedArgs {
|
|||
pub ex_units_mem: Option<u64>,
|
||||
#[serde(default)]
|
||||
pub ex_units_steps: Option<u64>,
|
||||
/// CRIT-1 audit fix (2026-05-12): bypass the configured
|
||||
/// `max_send_lovelace` hard cap on `dest_lovelace`. Required for
|
||||
/// large script-bootstrap mints (governor / stakes / proposal
|
||||
/// outputs that carry sizeable min-utxo + datum overhead).
|
||||
/// Bypass the `max_send_lovelace` cap on `dest_lovelace`. Needed
|
||||
/// for large script-bootstrap mints (governor / stakes / proposal
|
||||
/// outputs that carry sizable min-utxo + datum overhead).
|
||||
#[serde(default)]
|
||||
pub force: bool,
|
||||
}
|
||||
|
|
@ -721,8 +657,7 @@ pub struct MintUnsignedArgs {
|
|||
/// optional metadata for downstream signers.
|
||||
#[serde(default)]
|
||||
pub disclosed_signer_pkh_hex: Option<String>,
|
||||
/// CRIT-1 audit fix (2026-05-12): bypass the configured
|
||||
/// `max_send_lovelace` hard cap on `dest_lovelace`. Mirrors
|
||||
/// Bypass the `max_send_lovelace` cap on `dest_lovelace`. Mirrors
|
||||
/// `wallet_mint.force`.
|
||||
#[serde(default)]
|
||||
pub force: bool,
|
||||
|
|
@ -845,12 +780,10 @@ pub struct SignPartialArgs {
|
|||
/// partially signed by another party. The wallet's payment key
|
||||
/// gets appended as an additional VKeyWitness.
|
||||
pub cbor_hex: String,
|
||||
/// CRIT-1 audit fix (2026-05-12): bypass the configured
|
||||
/// `max_send_lovelace` hard cap. Before signing, the tool sums
|
||||
/// lovelace across every non-self output in the CBOR (anything
|
||||
/// not addressed to this wallet) and rejects if the total
|
||||
/// exceeds the cap. Pass `force=true` for cold-signed multi-sig
|
||||
/// flows where a treasury moves real value intentionally.
|
||||
/// Bypass the `max_send_lovelace` cap. Before signing, the tool
|
||||
/// sums lovelace across every non-self output in the CBOR; pass
|
||||
/// `force=true` for cold-signed multi-sig flows where a treasury
|
||||
/// moves real value intentionally.
|
||||
#[serde(default)]
|
||||
pub force: bool,
|
||||
}
|
||||
|
|
@ -1016,12 +949,11 @@ impl WalletService {
|
|||
if lovelace == 0 {
|
||||
return Err("lovelace must be > 0".into());
|
||||
}
|
||||
// AUDIT4-G2 fix: catch sub-min-utxo ada-only sends client-side
|
||||
// before the chain rejects them (saves a koios round-trip + the
|
||||
// user's mental model of "tx submitted" → "tx failed minutes later").
|
||||
// Asset-bearing sends have a dynamic min driven by asset count +
|
||||
// name lengths — let those reach chain so the real number is in
|
||||
// the error.
|
||||
// Catch sub-min-utxo ada-only sends client-side so the user
|
||||
// gets a clean error instead of "tx submitted" → "tx failed
|
||||
// minutes later". Asset-bearing sends have a dynamic min
|
||||
// (asset count + name length) — let those reach chain so
|
||||
// the real number is in the rejection.
|
||||
let default_min = ProtocolParams::default().min_utxo_lovelace;
|
||||
if assets.is_empty() && lovelace < default_min {
|
||||
return Err(format!(
|
||||
|
|
@ -1142,10 +1074,8 @@ impl WalletService {
|
|||
if lovelace == 0 {
|
||||
return Err("lovelace must be > 0".into());
|
||||
}
|
||||
// CRIT-1 audit fix (2026-05-12): apply the same cap as wallet_send
|
||||
// so the unsigned-build chain isn't a quiet bypass of the daemon's
|
||||
// value-safety policy. Defense-in-depth — the sign / submit
|
||||
// tools also re-check via enforce_cap_on_cbor.
|
||||
// Same cap as wallet_send so the unsigned-build chain isn't a
|
||||
// quiet bypass. Sign + submit re-check via enforce_cap_on_cbor.
|
||||
self.enforce_value_cap(lovelace, force)?;
|
||||
|
||||
let utxos = self
|
||||
|
|
@ -1225,11 +1155,9 @@ impl WalletService {
|
|||
}: SubmitSignedArgs,
|
||||
) -> Result<String, String> {
|
||||
let bytes = hex_decode(&signed_cbor_hex).map_err(|e| format!("decode: {e}"))?;
|
||||
// CRIT-1 audit fix (2026-05-12): scan the tx for outputs going
|
||||
// to non-self addresses and enforce the cap on the sum. Closes
|
||||
// the prompt-injection drain path where an attacker convinces
|
||||
// the LLM to hand-craft + sign + submit a tx that bypasses the
|
||||
// cap on the individual wallet_send tool.
|
||||
// Sum non-self outputs and enforce the cap. Closes the prompt-
|
||||
// injection drain path through hand-crafted + signed CBOR that
|
||||
// bypasses the per-tool wallet_send cap.
|
||||
self.enforce_cap_on_cbor(&bytes, force)?;
|
||||
self.inner
|
||||
.chain
|
||||
|
|
@ -1293,8 +1221,6 @@ impl WalletService {
|
|||
"dest_lovelace {dest_lovelace} below 1 ADA min — token-bearing UTXO will be rejected"
|
||||
));
|
||||
}
|
||||
// HIGH-1 audit fix: enforce hard cap on lovelace going to a
|
||||
// potentially-non-wallet destination.
|
||||
self.enforce_value_cap(dest_lovelace, force)?;
|
||||
|
||||
let utxos = self
|
||||
|
|
@ -1369,9 +1295,7 @@ impl WalletService {
|
|||
if user_lovelace < 1_000_000 || ref_lovelace < 1_000_000 {
|
||||
return Err("user_lovelace and ref_lovelace must each be ≥ 1 ADA".into());
|
||||
}
|
||||
// HIGH-1: cap on the total lovelace that leaves the wallet
|
||||
// toward non-self destinations. Sum the two outputs; if either
|
||||
// overflows, also reject.
|
||||
// Cap the total lovelace leaving the wallet (both outputs).
|
||||
let total = user_lovelace
|
||||
.checked_add(ref_lovelace)
|
||||
.ok_or("user_lovelace + ref_lovelace overflow")?;
|
||||
|
|
@ -1457,8 +1381,6 @@ impl WalletService {
|
|||
force,
|
||||
}: ScriptSpendArgs,
|
||||
) -> Result<String, String> {
|
||||
// HIGH-1: cap on payout_lovelace (the funds going to the
|
||||
// potentially-non-wallet payout address).
|
||||
self.enforce_value_cap(payout_lovelace, force)?;
|
||||
let version = match plutus_version.to_ascii_lowercase().as_str() {
|
||||
"v1" => PlutusVersion::V1,
|
||||
|
|
@ -1511,10 +1433,9 @@ impl WalletService {
|
|||
lovelace: locked_lovelace,
|
||||
};
|
||||
|
||||
// Plutus paths require the V3 cost model so the chain can
|
||||
// verify script_data_hash. Hardcoded preprod value from
|
||||
// koios epoch_params; future improvement is pulling fresh
|
||||
// from the chain on each call. (PLUTUS-4 audit fix.)
|
||||
// Plutus paths need the V3 cost model so the chain can verify
|
||||
// script_data_hash. Hardcoded preprod value from koios
|
||||
// epoch_params; eventually fetch fresh per call.
|
||||
let params = ProtocolParams {
|
||||
plutus_v3_cost_model: Some(PLUTUS_V3_COST_MODEL_PREPROD.to_vec()),
|
||||
..ProtocolParams::default()
|
||||
|
|
@ -1839,8 +1760,6 @@ impl WalletService {
|
|||
"dest_lovelace {dest_lovelace} below 1 ADA min for asset-bearing UTXO"
|
||||
));
|
||||
}
|
||||
// CRIT-1 audit fix (2026-05-12): cap the unsigned-mint output's
|
||||
// dest_lovelace, mirroring wallet_mint.
|
||||
self.enforce_value_cap(dest_lovelace, force)?;
|
||||
|
||||
// Resolve PolicySpec — caller-supplied JSON or wallet default.
|
||||
|
|
@ -1928,8 +1847,6 @@ impl WalletService {
|
|||
"dest_lovelace {dest_lovelace} below 1 ADA min for asset-bearing UTXO"
|
||||
));
|
||||
}
|
||||
// CRIT-1 audit fix (2026-05-12): cap the Plutus-mint
|
||||
// dest_lovelace, mirroring wallet_mint.
|
||||
self.enforce_value_cap(dest_lovelace, force)?;
|
||||
|
||||
let policy_cbor = resolve_policy_cbor_bytes(
|
||||
|
|
@ -2073,11 +1990,9 @@ impl WalletService {
|
|||
#[tool(aggr)] SignPartialArgs { cbor_hex, force }: SignPartialArgs,
|
||||
) -> Result<String, String> {
|
||||
let bytes = hex_decode(&cbor_hex).map_err(|e| format!("decode: {e}"))?;
|
||||
// CRIT-1 audit fix (2026-05-12): scan the unsigned/partial tx
|
||||
// for outputs going to non-self addresses and enforce the cap
|
||||
// on the sum BEFORE appending a witness. Closes the prompt-
|
||||
// injection drain path through the unsigned-build → sign →
|
||||
// submit chain (each step individually had no cap-awareness).
|
||||
// Sum non-self outputs and enforce the cap BEFORE appending a
|
||||
// witness — closes the prompt-injection drain through the
|
||||
// unsigned-build → sign → submit chain.
|
||||
self.enforce_cap_on_cbor(&bytes, force)?;
|
||||
let updated =
|
||||
add_witness(&self.inner.payment_key, &bytes).map_err(|e| format!("sign: {e}"))?;
|
||||
|
|
@ -4110,13 +4025,12 @@ impl WalletService {
|
|||
let wallet_utxos = pull_wallet_utxos(&self.inner.chain, &self.inner.address).await?;
|
||||
let (tip_slot, _tip_ms) = fetch_tip_slot_ms(self).await?;
|
||||
|
||||
// MED-2/3 fix (2026-05-09 audit): derive `validity_lower_ms`
|
||||
// from the slot via the Shelley constants, NOT from Koios's
|
||||
// `block_time*1000`. The chain reconstructs `lower` from
|
||||
// `valid_from_slot(slot)` via slot↔ms math; using Koios's
|
||||
// Derive `validity_lower_ms` from the slot via Shelley
|
||||
// constants, NOT from Koios's `block_time*1000`. The chain
|
||||
// reconstructs `lower` from the slot via slot↔ms math; Koios's
|
||||
// block_time can drift up to ~1s, which at the strict-`>`
|
||||
// validator boundary makes the off-chain preflight pass while
|
||||
// the chain rejects → fees + collateral burned.
|
||||
// validator boundary lets the off-chain preflight pass while
|
||||
// the chain rejects — burning fees + collateral.
|
||||
let validity_lower_slot = tip_slot;
|
||||
let validity_lower_ms = slot_to_posix_ms(cfg.network, validity_lower_slot)?;
|
||||
|
||||
|
|
@ -4184,7 +4098,7 @@ impl WalletService {
|
|||
let cfg = self.dao_cfg_for_escrow()?;
|
||||
let wallet_utxos = pull_wallet_utxos(&self.inner.chain, &self.inner.address).await?;
|
||||
let (tip_slot, _tip_ms) = fetch_tip_slot_ms(self).await?;
|
||||
// MED-2/3 fix: same slot-derived ms as Settle (see escrow_settle_unsigned).
|
||||
// Same slot-derived ms as Settle (see escrow_settle_unsigned).
|
||||
let validity_lower_slot = tip_slot;
|
||||
let validity_lower_ms = slot_to_posix_ms(cfg.network, validity_lower_slot)?;
|
||||
let validity_upper_slot = tip_slot + validity_window_seconds.unwrap_or(1800);
|
||||
|
|
@ -4493,16 +4407,15 @@ pub struct EscrowOpenUnsignedArgs {
|
|||
pub open_deadline_ms: i64,
|
||||
/// Veto-window length after Agree, in ms.
|
||||
pub lock_period_ms: i64,
|
||||
/// Optional pkh of the initial contributor (must equal party_a or party_b).
|
||||
/// **HIGH-2 fix 2026-05-09:** The builder now REJECTS `None` here — every
|
||||
/// escrow must have at least one initial contributor at open time.
|
||||
/// Sending `null` returns an error. Field stays `Option<String>` for
|
||||
/// schema-stability.
|
||||
/// pkh of the initial contributor (must equal party_a or party_b).
|
||||
/// Required — every escrow needs at least one contributor at open
|
||||
/// time. `None` returns an error. The field stays `Option<String>`
|
||||
/// for schema stability.
|
||||
#[serde(default)]
|
||||
pub initial_contributor_pkh_hex: Option<String>,
|
||||
/// Lovelace to lock at the escrow output. **MED-5 fix:** must clear
|
||||
/// 2_000_000 (≥2 ADA) — Conway-era inline-datum + script-address
|
||||
/// outputs need ~1.4-1.7 ADA min-utxo, with headroom.
|
||||
/// Lovelace to lock at the escrow output. Must clear 2_000_000
|
||||
/// (≥2 ADA) — Conway-era inline-datum + script-address outputs
|
||||
/// need ~1.4-1.7 ADA min-utxo, with headroom.
|
||||
pub initial_lovelace: u64,
|
||||
}
|
||||
|
||||
|
|
@ -4830,8 +4743,7 @@ mod sandbox_tests {
|
|||
use std::fs;
|
||||
use std::io::Write;
|
||||
|
||||
/// CRIT-2 audit fix (2026-05-12): files inside the sandbox root resolve
|
||||
/// cleanly via canonicalization.
|
||||
/// Files inside the sandbox root resolve cleanly via canonicalization.
|
||||
#[test]
|
||||
fn path_inside_sandbox_is_accepted() {
|
||||
let root = tempfile::tempdir().expect("tempdir");
|
||||
|
|
@ -4880,12 +4792,11 @@ mod sandbox_tests {
|
|||
);
|
||||
}
|
||||
|
||||
/// Adversarial-review fix: hardlinked files inside the sandbox
|
||||
/// must be refused. `canonicalize` resolves symlinks but NOT
|
||||
/// hardlinks — a hardlink IS the file (same inode, different
|
||||
/// directory entry). Without this, an attacker with daemon-uid
|
||||
/// write access could `link()` the encrypted key blob's inode
|
||||
/// into the sandbox and exfiltrate the bytes.
|
||||
/// Hardlinked files inside the sandbox must be refused.
|
||||
/// `canonicalize` resolves symlinks but not hardlinks (a hardlink
|
||||
/// IS the file — same inode), so without this guard an attacker
|
||||
/// with daemon-uid write access could `link()` the encrypted key
|
||||
/// blob's inode into the sandbox and exfiltrate the bytes.
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn hardlinked_file_inside_sandbox_is_rejected() {
|
||||
|
|
@ -4996,9 +4907,8 @@ mod cap_tests {
|
|||
(cbor, wallet_addr)
|
||||
}
|
||||
|
||||
/// CRIT-1: a tx whose non-self output sits below the cap returns 0
|
||||
/// total when the destination IS self (change-only); above cap the
|
||||
/// sum reflects the actual outbound lovelace.
|
||||
/// A tx whose only outputs are self-sends contributes 0 to the
|
||||
/// non-self total — self-moves don't count against the cap.
|
||||
#[test]
|
||||
fn sum_non_self_excludes_self_outputs() {
|
||||
// Send to the wallet's OWN address — every output is "self";
|
||||
|
|
@ -5016,9 +4926,8 @@ mod cap_tests {
|
|||
);
|
||||
}
|
||||
|
||||
/// CRIT-1: a tx sending lovelace to an UNRELATED address has its
|
||||
/// destination output reflected in the non-self sum, so the
|
||||
/// enforce_value_cap chokepoint can refuse to sign/submit it.
|
||||
/// Outbound lovelace to an unrelated address counts toward the
|
||||
/// non-self sum so the cap chokepoint can refuse to sign/submit.
|
||||
#[test]
|
||||
fn sum_non_self_counts_outbound_outputs() {
|
||||
// Send to a different derivation (account=0, index=1) of the
|
||||
|
|
@ -5038,22 +4947,11 @@ mod cap_tests {
|
|||
);
|
||||
}
|
||||
|
||||
/// Adversarial-review fix: `.sum::<u64>()` wraps on overflow in
|
||||
/// release builds. The cap check must NEVER silently roll over a
|
||||
/// u64 — a prompt-injection could craft CBOR with outputs summing
|
||||
/// past u64::MAX, wrap to a small value, pass the check. Now uses
|
||||
/// `checked_add` and returns Err on overflow. We test this via the
|
||||
/// free function with hand-constructed `OutputSummary` mock data
|
||||
/// rather than building a real CBOR (Cardano min-utxo + max-supply
|
||||
/// constraints make a real overflow tx impossible to build).
|
||||
///
|
||||
/// This test exercises `OutputSummary` from `aldabra-core::inspect`
|
||||
/// to confirm the type's `lovelace: u64` field is what we expect,
|
||||
/// and that the cap check refuses to silently roll over. A future
|
||||
/// refactor could expose a `compute_total` helper taking a slice
|
||||
/// of OutputSummary so we can test overflow in isolation; for now
|
||||
/// the test relies on documenting the invariant via a regression
|
||||
/// case in the free function below.
|
||||
/// The cap check must never silently roll over a u64 —
|
||||
/// `.sum::<u64>()` wraps on overflow in release builds. We use
|
||||
/// `checked_add` and return Err instead. Tested against
|
||||
/// synthetic `OutputSummary` values because Cardano min-utxo +
|
||||
/// max-supply make a real overflow CBOR impossible to build.
|
||||
#[test]
|
||||
fn sum_non_self_handles_overflow_safely() {
|
||||
// Build two synthetic OutputSummary mocks via the public type.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue