security: enforce max_send_lovelace + sandbox *_path args

Two CRIT findings from the 2026-05-12 Opus audit. Both are
mainnet-blocking against the aldabra-mainnet container.

CRIT-1 — cap-bypass via unsigned-build → sign_partial → submit chain.
Previously `wallet_send` / `wallet_mint` / `wallet_mint_cip68_nft` /
`wallet_script_spend` enforced `max_send_lovelace`, but the unsigned-
build tools + `wallet_sign_partial` + `wallet_submit_signed_tx` did
not. A prompt-injection that walked the cold-signer chain could drain
the wallet past the cap with zero policy enforcement.

Fix:
- `wallet_send_unsigned` / `wallet_mint_unsigned` /
  `wallet_plutus_mint_unsigned` now enforce the cap on the user-
  supplied destination lovelace, mirroring their signed equivalents.
  All three gain a `force: bool` arg with `#[serde(default)]`.
- `wallet_sign_partial` and `wallet_submit_signed_tx` decode the
  Conway tx CBOR, sum lovelace across every output whose address is
  NOT this wallet's own primary address, and enforce the cap on that
  total. Both gain `force: bool`. The chokepoint covers cold-signed
  multi-sig flows and any hand-built CBOR the daemon would otherwise
  blindly sign or submit.
- New free fn `sum_non_self_lovelace` is the unit-testable core of
  the chokepoint logic; `enforce_cap_on_cbor` wraps it.
- The sum uses `try_fold` + `checked_add` (NOT `.sum::<u64>()`) so a
  crafted CBOR that overflows `u64::MAX` fails the check instead of
  wrapping silently in release builds.

CRIT-2 — path traversal via `reference_script_path` and
`policy_cbor_path`. Previously the tools called `std::fs::read_to_
string(p)` on any path the LLM passed. The MCP daemon runs as the
same user that owns `$ALDABRA_DATA/mnemonic.age` /
`$ALDABRA_DATA/root-xprv.age`. Decode-error messages included the
hex_decode position offset — a small but real information leak about
non-hex file structure.

Fix:
- New `Config::safe_reads_root` field (default `$ALDABRA_DATA/scripts/`,
  override via `ALDABRA_SAFE_READS_ROOT` env or TOML).
- New `assert_inside_sandbox` helper canonicalize()s both the root and
  the user-supplied path, then enforces `starts_with`. Rejects
  outside-root paths, `..`-traversal, and nonexistent paths with
  generic messages.
- Hardlink-rejection: post-canonicalize, stat the file and refuse if
  `nlink > 1`. `canonicalize` resolves symlinks but NOT hardlinks (a
  hardlink IS the file — same inode, different directory entry), so
  without this check an attacker with daemon-uid write access could
  plant a hardlink to the encrypted key blob inside the sandbox and
  exfiltrate bytes through the read path.
- `resolve_ref_script_bytes` + `resolve_policy_cbor_bytes` + the
  `resolve_validator_required` wrapper used by all 5 escrow spend
  tools take `&Path` and route through the sandbox.
- Error messages on hex_decode failures no longer carry the path
  string or byte-offset position — return a constant "contents are
  not valid hex" instead.
- `main.rs` creates the sandbox root with 0o700 perms at startup if
  missing. chmod errors are surfaced (not swallowed) so a broken
  filesystem doesn't silently fall back to umask 0o755.
- README documents the new `ALDABRA_SAFE_READS_ROOT` env var alongside
  `ALDABRA_MAX_SEND_LOVELACE` (also previously undocumented).

Tests (243 → 253, +10):
- 5 sandbox tests: accept-inside, reject-outside, reject-dotdot,
  reject-nonexistent, reject-hardlink.
- 1 non-hex regression: constant message (no byte-offset leak).
- 3 cap tests: self-send → 0 non-self total, outbound counts,
  overflow → Err (regression for the prompt-injection `u64::MAX`
  wraparound attempt).
- 1 garbage-CBOR test: clean error.

No new clippy warnings, no new fmt drift, `cargo audit` unchanged
(0 CVEs, 2 transitive unmaintained warnings).

Adversarial review of the first draft (3 Opus reviewers) caught the
u64 overflow, the hardlink bypass, and the swallowed chmod error.
This commit is contained in:
Sulkta 2026-05-12 11:12:51 -07:00
parent 564ba3ccb5
commit 4e4becd0bb
6 changed files with 567 additions and 20 deletions

View file

@ -56,9 +56,41 @@ async fn run() -> Result<()> {
account = cfg.account,
index = cfg.index,
data_dir = %cfg.data_dir.display(),
safe_reads_root = %cfg.safe_reads_root.display(),
"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.
if !cfg.safe_reads_root.exists() {
std::fs::create_dir_all(&cfg.safe_reads_root).map_err(|e| {
anyhow::anyhow!(
"create safe_reads_root {}: {e}",
cfg.safe_reads_root.display()
)
})?;
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
std::fs::set_permissions(&cfg.safe_reads_root, std::fs::Permissions::from_mode(0o700))
.map_err(|e| {
anyhow::anyhow!(
"chmod 0o700 safe_reads_root {}: {e}",
cfg.safe_reads_root.display()
)
})?;
}
}
// CLI mode flags — all out-of-band, before the MCP transport
// takes over stdio.
//
@ -132,6 +164,7 @@ async fn run() -> Result<()> {
stake_key,
cfg.max_send_lovelace,
cfg.data_dir.clone(),
cfg.safe_reads_root.clone(),
);
let server = service
.serve(stdio())