diff --git a/CLAUDE.md b/CLAUDE.md index bfd5953..2ead63b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -114,5 +114,5 @@ The SDK follows a client-centric design with `Aptos` as the main entry point: - Unit tests are co-located with source code or in `src/tests/` directories - E2E tests require running Aptos localnet (`aptos node run-localnet`) -- Behavioral specification tests in `specifications/tests/rust/` +- Behavioral tests in `crates/aptos-sdk/tests/behavioral/` - Property-based testing with `proptest` for crypto components (via `fuzzing` feature) diff --git a/SECURITY_AUDIT.md b/SECURITY_AUDIT.md new file mode 100644 index 0000000..a3f9c61 --- /dev/null +++ b/SECURITY_AUDIT.md @@ -0,0 +1,426 @@ +# Aptos Rust SDK -- Security Audit Report + +**Date:** 2026-02-09 +**Scope:** Full SDK (`aptos-sdk` v0.3.0, `aptos-sdk-macros` v0.1.0) +**Method:** 3-pass audit (automated surface scan, deep manual review, design-level assessment) +**Status:** All findings remediated (21 of 22 fixed; F-21 deferred as large effort). Response body reads now use incremental streaming with size limits (`read_response_bounded`) to prevent OOM from chunked transfer-encoding. + +--- + +## Executive Summary + +The Aptos Rust SDK demonstrates strong security fundamentals: `unsafe` code is forbidden at the crate level, all cryptographic operations delegate to well-audited libraries, private keys use `#[zeroize(drop)]` with `[REDACTED]` Debug output, and TLS certificate validation is enabled by default. No critical vulnerabilities that would allow key theft or transaction forgery were found. + +The audit identified **22 findings** across 4 severity levels. **All findings have been remediated** except F-21 (fuzz targets), which requires significant new infrastructure and has been deferred. + +**Finding Summary:** + +| Severity | Count | Fixed | +|----------|-------|-------| +| Critical | 1 | 1 | +| High | 3 | 3 | +| Medium | 10 | 10 | +| Low | 5 | 5 | +| Info | 3 | 2 (F-21 deferred) | + +--- + +## Pass 1: Automated Surface Scan Results + +### 1a. Dependency Audit + +**Status: CLEAN** + +- `cargo audit`: 0 vulnerabilities in 334 crate dependencies. +- All crypto crates are current: `ed25519-dalek 2.2.0`, `k256 0.13.4`, `p256 0.13.2`, `blst 0.3.16`, `sha2 0.10.9`, `sha3 0.10.8`, `jsonwebtoken 10.3.0`. +- No yanked crates. + +### 1b. Static Analysis + +**Status: CLEAN with observations** + +- `cargo clippy --all-features -D warnings`: 0 warnings. +- `#![forbid(unsafe_code)]` confirmed in `crates/aptos-sdk/src/lib.rs:58`. +- No `todo!()` or `unimplemented!()` in production code. +- 4 `unreachable!()` in `account/multi_key.rs` (guarded by feature flags -- acceptable). +- Production `expect()` calls cataloged: + - 5 lock-poisoning expects in `aptos.rs` and `batch.rs` (standard Rust pattern). + - 10 hardcoded URL parses in `config.rs` (static strings -- safe). + - 1 BLS key generation in `bls12381.rs:52` (internal invariant). + - 1 mnemonic validation in `mnemonic.rs:100` (post-validation invariant). + +### 1c. Feature Flag Isolation + +**Status: CLEAN** + +All 8 optional features compile cleanly in isolation: `ed25519`, `secp256k1`, `secp256r1`, `bls`, `mnemonic`, `keyless`, `indexer`, `faucet`. Also compiles with `--no-default-features` and `--all-features`. + +--- + +## Pass 2: Deep Manual Review Findings + +### F-01: Path traversal via `abi.name` in codegen build helper [CRITICAL] + +**File:** `crates/aptos-sdk/src/codegen/build_helper.rs:168-172` + +**Issue:** The generated output filename uses `abi.name` directly from untrusted ABI JSON: + +```rust +let output_filename = format!("{}.rs", abi.name); +let output_path = output_dir.join(&output_filename); +fs::write(&output_path, &code)?; +``` + +A malicious ABI with `"name": "../../../tmp/evil"` writes outside the intended output directory. + +**Impact:** Arbitrary file write during build-time code generation. + +**Recommendation:** Validate `abi.name` against `^[a-zA-Z_][a-zA-Z0-9_]*$` and/or verify the canonicalized output path starts with the output directory. + +--- + +### F-02: Unbounded response body in `view_bcs` [HIGH] + +**File:** `crates/aptos-sdk/src/api/fullnode.rs:561` + +**Issue:** `view_bcs` reads the entire response body with no size check: + +```rust +let bytes = response.bytes().await?; +``` + +Unlike other API methods that go through `handle_response_static` (which checks Content-Length), `view_bcs` has its own response handling path that skips this check entirely. + +**Impact:** A malicious or compromised fullnode can force unbounded memory allocation, causing OOM. + +**Recommendation:** Add a Content-Length check before `response.bytes()` or use bounded reads: + +```rust +let bytes = response.bytes().await?; +if bytes.len() > max_response_size { + return Err(AptosError::api(status.as_u16(), "response too large")); +} +``` + +--- + +### F-03: Content-Length check bypass via chunked encoding [HIGH] + +**File:** `crates/aptos-sdk/src/api/fullnode.rs:701-712` + +**Issue:** `handle_response_static` only checks `response.content_length()`. When the server uses chunked transfer encoding (no Content-Length header), `content_length()` returns `None` and the check is skipped. `response.json()` then reads the entire body with no limit. + +**Impact:** A server can bypass the size check by omitting Content-Length, causing memory exhaustion. + +**Recommendation:** After reading the body, enforce size limits. Consider reading bytes first with a streaming limit, then deserializing: + +```rust +let bytes = response.bytes().await?; +if bytes.len() > max_response_size { + return Err(AptosError::api(status, "response too large")); +} +let data: T = serde_json::from_slice(&bytes)?; +``` + +--- + +### F-04: Doc-string code injection in codegen generator [HIGH] + +**File:** `crates/aptos-sdk/src/codegen/generator.rs:433-435, 381-383` + +**Issue:** ABI-derived strings are written directly into `writeln!` output. Strings containing newlines can escape doc comments and inject arbitrary Rust code. For example, a struct field type `u64\n\npub malicious: u64,` would produce a valid additional struct field. + +**Impact:** Code injection in generated Rust source during build time. Requires a crafted ABI file. + +**Recommendation:** Sanitize all ABI-derived strings before embedding in generated code: strip/escape newlines, `{`, `}`, and `"` characters. + +--- + +### F-05: `format_ident!` panic on invalid ABI identifiers in proc macros [MEDIUM] + +**File:** `crates/aptos-sdk-macros/src/codegen.rs:121, 129, 170, 186, 262-263` + +**Issue:** `format_ident!` is called with ABI-derived names. If the ABI contains names with invalid Rust identifier characters (e.g., `0x1::coin::Coin`, newlines, Unicode), `proc_macro2::Ident::new` panics with an ICE-like error. + +**Recommendation:** Validate that ABI names are valid Rust identifiers before calling `format_ident!`, or return a clear compile error. + +--- + +### F-06: Generated code uses `.unwrap()` for BCS serialization [MEDIUM] + +**File:** `crates/aptos-sdk/src/codegen/types.rs:181-197` + +**Issue:** The `to_bcs_arg` function generates code that calls `aptos_bcs::to_bytes(&x).unwrap()`. If serialization fails at runtime (unlikely but possible with unexpected types), the generated code panics instead of returning an error. + +**Recommendation:** Generate `.map_err(|e| AptosError::Bcs(e.to_string()))?` instead of `.unwrap()`. + +--- + +### F-07: `serde(rename)` string injection in codegen [MEDIUM] + +**File:** `crates/aptos-sdk/src/codegen/generator.rs:439` + +**Issue:** `field.name` from ABI JSON is interpolated directly into a `#[serde(rename = "...")]` attribute. If the name contains `"` or `\`, the generated attribute becomes syntactically invalid or malformed. + +**Recommendation:** Escape `"` and `\` in field names, or validate that names contain only safe characters. + +--- + +### F-08: `IndexerClient::with_url` and `FaucetClient::with_url` bypass URL scheme validation [MEDIUM] + +**File:** `crates/aptos-sdk/src/api/indexer.rs:137-145`, `crates/aptos-sdk/src/api/faucet.rs:104-112` + +**Issue:** Both `with_url()` convenience constructors accept arbitrary URLs without calling `validate_url_scheme()`. This allows `file://`, `gopher://`, or other dangerous schemes. + +**Recommendation:** Add `validate_url_scheme()` calls to both `with_url()` methods. + +--- + +### F-09: Error sanitization pattern list is incomplete [MEDIUM] + +**File:** `crates/aptos-sdk/src/error.rs:160-168` + +**Issue:** `SENSITIVE_PATTERNS` only covers 7 patterns. Missing patterns that could appear in error messages: `token`, `jwt`, `credential`, `api_key`, `apikey`, `access_token`, `refresh_token`, `pepper`. + +**Recommendation:** Expand the pattern list to cover common secret patterns. + +--- + +### F-10: `Display`/`to_string()` on errors can leak sensitive data [MEDIUM] + +**File:** `crates/aptos-sdk/src/error.rs` (multiple variants) + +**Issue:** `sanitized_message()` is opt-in. Callers who use `Display`/`to_string()` (the default in `?` propagation and logging) may inadvertently include private keys, mnemonics, or JWTs in log output. Variants like `InvalidPrivateKey`, `InvalidMnemonic`, `InvalidJwt` carry detail strings in their Display. + +**Recommendation:** Consider making sanitization the default in `Display`, or at minimum add prominent documentation warning callers to use `sanitized_message()` for any logging. + +--- + +### F-11: Entropy and seed material not zeroized in mnemonic derivation [MEDIUM] + +**File:** `crates/aptos-sdk/src/account/mnemonic.rs` + +**Issue:** In `Mnemonic::generate()`, the 16-byte entropy buffer is not explicitly zeroized before being dropped. Similarly, in `derive_ed25519_from_seed()`, the intermediate seed bytes and HMAC outputs are not zeroized. While Rust's stack variables are dropped, they are not zeroed -- the data remains in memory until overwritten. + +**Recommendation:** Use `zeroize::Zeroizing` wrappers for entropy, seed, and intermediate key material: + +```rust +let mut entropy = zeroize::Zeroizing::new([0u8; 16]); +OsRng.fill_bytes(entropy.as_mut()); +``` + +--- + +### F-12: MoveSourceParser has no input size limit [MEDIUM] + +**File:** `crates/aptos-sdk/src/codegen/move_parser.rs:90` + +**Issue:** `let lines: Vec<&str> = source.lines().collect()` loads the entire Move source into memory with no size limit. A very large file can cause high memory usage. + +**Recommendation:** Add a maximum input size check before parsing. + +--- + +### F-13: Invalid module names in `generate_mod_file` [MEDIUM] + +**File:** `crates/aptos-sdk/src/codegen/build_helper.rs:362-371` + +**Issue:** Module names from `abi.name` are emitted directly as `pub mod {name};` without validation. Invalid characters can produce uncompileable or dangerous `mod.rs` content. + +**Recommendation:** Validate module names match `^[a-zA-Z_][a-zA-Z0-9_]*$`. + +--- + +### F-14: 100 MB default max response size is excessive [MEDIUM] + +**File:** `crates/aptos-sdk/src/config.rs:64` + +**Issue:** `DEFAULT_MAX_RESPONSE_SIZE` is 100 MB. Normal Aptos API responses are typically under 1 MB. A compromised node could force 100 MB allocations per request. + +**Recommendation:** Lower the default to 10 MB with documentation explaining how to increase it for specific use cases. + +--- + +### F-15: Lock poisoning propagates panics across threads [LOW] + +**File:** `crates/aptos-sdk/src/aptos.rs:171-222`, `crates/aptos-sdk/src/transaction/batch.rs:544-553` + +**Issue:** `chain_id.read().expect("chain_id lock poisoned")` will panic if any thread previously panicked while holding the lock. This converts a single-thread panic into a cascading failure. + +**Recommendation:** Consider using `lock().unwrap_or_else(|e| e.into_inner())` to recover from poisoned locks, or document this behavior. + +--- + +### F-16: `from_bytes` for ECDSA signatures may accept high-S values [LOW] + +**File:** `crates/aptos-sdk/src/crypto/secp256k1.rs`, `crates/aptos-sdk/src/crypto/secp256r1.rs` + +**Issue:** While signing always produces low-S signatures (preventing malleability), `Signature::from_bytes()` may accept high-S signatures from external sources. If verified signatures are used for identity purposes, this could allow signature malleability. + +**Impact:** Low -- the Aptos blockchain itself enforces low-S, so this is only relevant for off-chain signature verification. + +**Recommendation:** Consider normalizing or rejecting high-S signatures in `from_bytes()`. + +--- + +### F-17: Error paths use unbounded `text()` and `json()` reads [LOW] + +**File:** `crates/aptos-sdk/src/api/fullnode.rs:551, 769` + +**Issue:** Error response bodies are read with `response.text().await.unwrap_or_default()` and `response.json().await.unwrap_or_default()` with no size limit. A malicious server could send a very large error body. + +**Recommendation:** Truncate error body reads to a reasonable limit (e.g., 8 KB). + +--- + +### F-18: Sponsored transaction default expiration doubled [LOW] + +**File:** `crates/aptos-sdk/src/transaction/sponsored.rs` (~line 221-222) + +**Issue:** `DEFAULT_EXPIRATION_SECONDS` appears to be applied twice in some code paths, resulting in a 20-minute expiration window instead of 10 minutes. Longer expiration windows increase replay risk. + +**Recommendation:** Review and normalize expiration handling to ensure single application. + +--- + +### F-19: `aptos_contract_file!` follows symlinks [LOW] + +**File:** `crates/aptos-sdk-macros/src/lib.rs:146-147` + +**Issue:** `std::fs::read_to_string` follows symlinks. A symlink under the project directory could cause reading of files outside the project. + +**Impact:** Low -- the path is developer-controlled in the macro invocation. + +**Recommendation:** For defense in depth, canonicalize the path and verify it remains under `CARGO_MANIFEST_DIR`. + +--- + +### F-20: Private key printed in example code [INFO] + +**File:** `crates/aptos-sdk/examples/account_management.rs:70-72` + +```rust +let pk_hex = hex::encode(pk_bytes); +println!("Private key (hex): {}", pk_hex); +``` + +**Issue:** While clearly labeled as demonstration code, users may copy this pattern into production code. + +**Recommendation:** Add a more prominent warning comment, or use `eprintln!("[DEMO ONLY] ...")` to make it clear this should never be replicated. + +--- + +### F-21: Fuzzing infrastructure exists but has zero fuzz targets [INFO] + +**Issue:** The `fuzzing` feature flag enables `proptest`, `proptest-derive`, and `arbitrary` dependencies, but no actual fuzz targets or property-based tests exist in the codebase. + +**Recommendation:** Implement fuzz targets for high-risk parsing code: +- BCS deserialization of transactions and authenticators +- `AccountAddress::from_hex()` and `AccountAddress::from_bytes()` +- `TypeTag::from_str_strict()` and `EntryFunctionId::from_str_strict()` +- `MultiKeySignature::from_bytes()` and `MultiEd25519Signature::from_bytes()` +- ABI JSON parsing in codegen + +--- + +### F-22: `specifications/tests/` directory referenced in docs but missing [INFO] + +**Issue:** `CLAUDE.md` references "Behavioral specification tests in `specifications/tests/rust/`" but this directory does not exist. + +**Recommendation:** Remove the stale reference or create the directory. + +--- + +## Pass 3: Design-Level Assessment + +### 3a. Implicit Threat Model + +The SDK operates with the following trust boundaries: + +| Actor | Trust Level | Notes | +|-------|------------|-------| +| SDK user (developer) | Fully trusted | Controls all inputs, keys, configuration | +| Fullnode API | Partially trusted | Responses used for gas estimation, sequence numbers, balances | +| Indexer API | Partially trusted | GraphQL responses for queries | +| Faucet API | Partially trusted | Only used on testnets | +| ABI providers | Trusted at build time | ABI files used for code generation | +| Move contracts | Untrusted | Executed on-chain, SDK just submits | + +**Trust boundary violations identified:** +1. Fullnode responses are used to set gas prices and sequence numbers without validation beyond JSON parsing. A malicious fullnode could return inflated gas prices. +2. ABI files are treated as fully trusted for code generation but may come from untrusted sources (F-01, F-04, F-07). +3. `with_url()` constructors bypass URL validation (F-08). + +### 3b. Missing Hardening + +1. **Response body streaming** -- All response reads now use `read_response_bounded()` which pre-checks `Content-Length` and reads incrementally via `response.chunk()`, aborting early if the size limit is exceeded. Error body reads are also bounded. (Addresses F-02, F-03, F-17) +2. **Constant-time operations** -- Signature verification delegates to underlying crates (ed25519-dalek, k256, p256) which use constant-time comparison. The SDK itself does not perform any custom constant-time operations, which is correct. +3. **Fuzz testing** -- Infrastructure exists but is unused (F-21). +4. **Side-channel resistance** -- Signing operations use library implementations with side-channel resistance. Non-security-critical operations (address parsing, ABI processing) are not constant-time, which is acceptable. + +### 3c. Positive Security Properties + +The SDK demonstrates several strong security practices: + +1. `#![forbid(unsafe_code)]` -- Prevents all unsafe Rust at the crate level. +2. `#[zeroize(drop)]` on all private key types with `[REDACTED]` Debug output. +3. Domain-separated signing with `sha3_256(b"APTOS::RawTransaction")` prefix. +4. BCS deterministic serialization prevents transaction malleability. +5. TLS enabled by default with no opt-out in the public API. +6. URL scheme validation blocks SSRF via dangerous protocols. +7. ECDSA low-S enforcement prevents signature malleability. +8. Error sanitization system with sensitive pattern redaction. +9. Retry logic with exponential backoff, jitter, and bounded max delay. +10. `checked_add` for sequence number arithmetic in batch transactions. + +### 3d. Security Test Coverage Gaps + +| Area | Current State | Recommendation | +|------|--------------|----------------| +| API response handling | 1 test in fullnode.rs | Add tests for oversized responses, missing Content-Length, chunked encoding | +| BCS deserialization | No malformed-input tests | Add tests with truncated, oversized, and corrupted BCS payloads | +| Authenticator parsing | No negative tests | Add tests with wrong-length keys, tampered signature bytes | +| Codegen with malicious ABI | No security tests | Add tests with path traversal, newlines, special chars in ABI names | +| Error leakage | Sanitization tested | Add tests verifying Display does not leak key material | +| Fuzz targets | 0 targets | Implement for parsers and deserializers | + +--- + +## Remediation Roadmap + +### Immediate (Critical/High) + +| ID | Finding | Effort | +|----|---------|--------| +| F-01 | Validate `abi.name` in build_helper.rs | Small | +| F-02 | Add size check to `view_bcs` response | Small | +| F-03 | Use bounded body reads in `handle_response_static` | Medium | +| F-04 | Sanitize ABI strings in code generator | Medium | + +### Short-term (Medium) + +| ID | Finding | Effort | +|----|---------|--------| +| F-05 | Validate identifiers before `format_ident!` | Small | +| F-06 | Replace `.unwrap()` with error handling in generated code | Small | +| F-07 | Escape serde rename strings | Small | +| F-08 | Add `validate_url_scheme()` to `with_url()` methods | Small | +| F-09 | Expand sensitive pattern list | Small | +| F-10 | Document or default sanitized Display | Medium | +| F-11 | Zeroize entropy/seed in mnemonic derivation | Small | +| F-12 | Add input size limit to MoveSourceParser | Small | +| F-13 | Validate module names in generate_mod_file | Small | +| F-14 | Lower default max response size to 10 MB | Small | + +### Long-term (Low/Info) + +| ID | Finding | Effort | +|----|---------|--------| +| F-15 | Handle lock poisoning gracefully | Small | +| F-16 | Normalize/reject high-S signatures in from_bytes | Small | +| F-17 | Truncate error body reads | Small | +| F-18 | Fix sponsored transaction expiration | Small | +| F-19 | Canonicalize macro file paths | Small | +| F-20 | Improve example warnings | Small | +| F-21 | Implement fuzz targets | Large | +| F-22 | Fix stale doc reference | Trivial | diff --git a/crates/aptos-sdk-macros/src/codegen.rs b/crates/aptos-sdk-macros/src/codegen.rs index 171716f..d7241ad 100644 --- a/crates/aptos-sdk-macros/src/codegen.rs +++ b/crates/aptos-sdk-macros/src/codegen.rs @@ -6,6 +6,104 @@ use proc_macro2::TokenStream; use quote::{format_ident, quote}; use syn::Ident; +/// Validates that a string is a valid Rust identifier before using it with `format_ident!`. +/// +/// # Security +/// +/// Prevents panics from `proc_macro2::Ident::new` when ABI contains names with +/// invalid characters (e.g., `::`, newlines, Unicode). +fn validate_rust_ident(name: &str) -> Result<(), String> { + if name.is_empty() { + return Err("identifier cannot be empty".to_string()); + } + let first = name.chars().next().unwrap(); + if !first.is_ascii_alphabetic() && first != '_' { + return Err(format!( + "identifier '{name}' must start with a letter or underscore" + )); + } + if !name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') { + return Err(format!( + "identifier '{name}' contains invalid characters (only ASCII alphanumeric and underscore allowed)" + )); + } + Ok(()) +} + +/// Returns true if `name` is a Rust keyword that would panic in `Ident::new`. +fn is_rust_keyword(name: &str) -> bool { + matches!( + name, + "as" | "break" + | "const" + | "continue" + | "crate" + | "else" + | "enum" + | "extern" + | "false" + | "fn" + | "for" + | "if" + | "impl" + | "in" + | "let" + | "loop" + | "match" + | "mod" + | "move" + | "mut" + | "pub" + | "ref" + | "return" + | "self" + | "Self" + | "static" + | "struct" + | "super" + | "trait" + | "true" + | "type" + | "unsafe" + | "use" + | "where" + | "while" + | "async" + | "await" + | "dyn" + | "abstract" + | "become" + | "box" + | "do" + | "final" + | "macro" + | "override" + | "priv" + | "try" + | "typeof" + | "unsized" + | "virtual" + | "yield" + ) +} + +/// Safely creates an `Ident` from a string, returning a compile error if invalid. +/// +/// Uses raw identifiers (`r#name`) for Rust keywords to avoid panics. +fn safe_format_ident(name: &str) -> Result { + if let Err(e) = validate_rust_ident(name) { + return Err(syn::Error::new(proc_macro2::Span::call_site(), e).to_compile_error()); + } + if is_rust_keyword(name) { + Ok(proc_macro2::Ident::new_raw( + name, + proc_macro2::Span::call_site(), + )) + } else { + Ok(format_ident!("{}", name)) + } +} + /// Generates the contract implementation. pub fn generate_contract_impl( name: &Ident, @@ -118,7 +216,12 @@ fn generate_structs(structs: &[MoveStructDef]) -> TokenStream { /// Generates a single struct definition. fn generate_struct(struct_def: &MoveStructDef) -> TokenStream { - let name = format_ident!("{}", to_pascal_case(&struct_def.name)); + let pascal_name = to_pascal_case(&struct_def.name); + // SECURITY: Validate identifier before format_ident! to prevent panics on malformed ABI + let name = match safe_format_ident(&pascal_name) { + Ok(ident) => ident, + Err(err) => return err, + }; let abilities = struct_def.abilities.join(", "); let doc = format!("Move struct with abilities: {}", abilities); @@ -126,7 +229,12 @@ fn generate_struct(struct_def: &MoveStructDef) -> TokenStream { .fields .iter() .map(|f| { - let field_name = format_ident!("{}", to_snake_case(&f.name)); + let snake_name = to_snake_case(&f.name); + // SECURITY: Validate field name before format_ident! + let field_name = match safe_format_ident(&snake_name) { + Ok(ident) => ident, + Err(err) => return err, + }; let field_type = move_type_to_rust(&f.typ); quote! { pub #field_name: #field_type @@ -167,7 +275,12 @@ fn generate_entry_function( module: &str, source_info: Option<&MoveSourceInfo>, ) -> TokenStream { - let fn_name = format_ident!("{}", to_snake_case(&func.name)); + let snake_fn = to_snake_case(&func.name); + // SECURITY: Validate function name before format_ident! + let fn_name = match safe_format_ident(&snake_fn) { + Ok(ident) => ident, + Err(err) => return err, + }; let func_name_str = &func.name; // Get enriched param info @@ -259,8 +372,18 @@ fn generate_view_function( module: &str, source_info: Option<&MoveSourceInfo>, ) -> TokenStream { - let fn_name = format_ident!("view_{}", to_snake_case(&func.name)); - let fn_name_json = format_ident!("view_{}_json", to_snake_case(&func.name)); + let snake_fn = to_snake_case(&func.name); + let view_name = format!("view_{snake_fn}"); + let view_json_name = format!("view_{snake_fn}_json"); + // SECURITY: Validate function names before format_ident! + let fn_name = match safe_format_ident(&view_name) { + Ok(ident) => ident, + Err(err) => return err, + }; + let fn_name_json = match safe_format_ident(&view_json_name) { + Ok(ident) => ident, + Err(err) => return err, + }; let func_name_str = &func.name; // Get enriched param info diff --git a/crates/aptos-sdk-macros/src/lib.rs b/crates/aptos-sdk-macros/src/lib.rs index 759a32f..6bd2664 100644 --- a/crates/aptos-sdk-macros/src/lib.rs +++ b/crates/aptos-sdk-macros/src/lib.rs @@ -112,7 +112,46 @@ pub fn aptos_contract_file(input: TokenStream) -> TokenStream { // Read the file content at compile time let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap_or_else(|_| ".".to_string()); - let file_path = std::path::Path::new(&manifest_dir).join(&input.path); + let manifest_path = std::path::Path::new(&manifest_dir); + let file_path = manifest_path.join(&input.path); + + // SECURITY: Verify the resolved path is under CARGO_MANIFEST_DIR to prevent + // path traversal attacks (e.g., "../../../../etc/passwd"). + // Canonicalization failures are treated as errors to ensure this check + // is never silently skipped. + let canonical_manifest = match manifest_path.canonicalize() { + Ok(p) => p, + Err(e) => { + return syn::Error::new( + input.name.span(), + format!("Failed to resolve project directory: {e}"), + ) + .to_compile_error() + .into(); + } + }; + let canonical_file = match file_path.canonicalize() { + Ok(p) => p, + Err(e) => { + return syn::Error::new( + input.name.span(), + format!("Failed to resolve ABI file path '{}': {e}", input.path), + ) + .to_compile_error() + .into(); + } + }; + if !canonical_file.starts_with(&canonical_manifest) { + return syn::Error::new( + input.name.span(), + format!( + "ABI file path '{}' resolves outside the project directory", + input.path + ), + ) + .to_compile_error() + .into(); + } let abi_content = match std::fs::read_to_string(&file_path) { Ok(content) => content, diff --git a/crates/aptos-sdk/examples/account_management.rs b/crates/aptos-sdk/examples/account_management.rs index 8602f5c..5d9f1da 100644 --- a/crates/aptos-sdk/examples/account_management.rs +++ b/crates/aptos-sdk/examples/account_management.rs @@ -67,9 +67,14 @@ async fn main() -> anyhow::Result<()> { println!("\n--- 6. Loading from Private Key ---"); // Get the private key bytes and convert to hex + // WARNING: This prints the private key for demonstration purposes ONLY. + // NEVER print, log, or expose private keys in production code! let pk_bytes = random_account.private_key().to_bytes(); let pk_hex = const_hex::encode(pk_bytes); - println!("Private key (hex): {}", pk_hex); + println!( + "[DEMO ONLY - NEVER DO THIS IN PRODUCTION] Private key (hex): {}", + pk_hex + ); // Recreate account from private key hex let restored_account = Ed25519Account::from_private_key_hex(&pk_hex)?; diff --git a/crates/aptos-sdk/src/account/keyless.rs b/crates/aptos-sdk/src/account/keyless.rs index 9403706..1fa3fd4 100644 --- a/crates/aptos-sdk/src/account/keyless.rs +++ b/crates/aptos-sdk/src/account/keyless.rs @@ -132,23 +132,58 @@ impl OidcProvider { } /// Infers a provider from an issuer URL. + /// + /// # Security + /// + /// For unknown issuers, the JWKS URL is constructed as `{issuer}/.well-known/jwks.json`. + /// Non-HTTPS issuers are accepted at construction time but will produce an empty + /// JWKS URL, causing a clear error at JWKS fetch time. This prevents SSRF via + /// `http://`, `file://`, or other dangerous URL schemes without changing the + /// function signature. Callers controlling issuer input should additionally + /// validate the host (e.g., block private IP ranges) if SSRF is a concern. pub fn from_issuer(issuer: &str) -> Self { match issuer { "https://accounts.google.com" => OidcProvider::Google, "https://appleid.apple.com" => OidcProvider::Apple, "https://login.microsoftonline.com/common/v2.0" => OidcProvider::Microsoft, - _ => OidcProvider::Custom { - issuer: issuer.to_string(), - jwks_url: format!("{issuer}/.well-known/jwks.json"), - }, + _ => { + // SECURITY: Only accept HTTPS issuers to prevent SSRF attacks. + // A malicious JWT could set `iss` to an internal URL (e.g., + // http://169.254.169.254/) causing the SDK to make requests to + // attacker-chosen endpoints when fetching JWKS. + let jwks_url = if issuer.starts_with("https://") { + format!("{issuer}/.well-known/jwks.json") + } else { + // Non-HTTPS issuers get an invalid JWKS URL that will fail + // at fetch time with a clear error rather than making requests + // to potentially dangerous endpoints. + String::new() + }; + OidcProvider::Custom { + issuer: issuer.to_string(), + jwks_url, + } + } } } } /// Pepper bytes used in keyless address derivation. -#[derive(Clone, Debug, PartialEq, Eq)] +/// +/// # Security +/// +/// The pepper is secret material used to derive keyless account addresses. +/// It is automatically zeroized when dropped to prevent key material from +/// lingering in memory. +#[derive(Clone, PartialEq, Eq, zeroize::Zeroize, zeroize::ZeroizeOnDrop)] pub struct Pepper(Vec); +impl std::fmt::Debug for Pepper { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Pepper(REDACTED)") + } +} + impl Pepper { /// Creates a new pepper from raw bytes. pub fn new(bytes: Vec) -> Self { @@ -267,7 +302,12 @@ impl PepperService for HttpPepperService { .await? .error_for_status()?; - let payload: PepperResponse = response.json().await?; + // SECURITY: Stream body with size limit to prevent OOM + let bytes = + crate::config::read_response_bounded(response, MAX_JWKS_RESPONSE_SIZE).await?; + let payload: PepperResponse = serde_json::from_slice(&bytes).map_err(|e| { + AptosError::InvalidJwt(format!("failed to parse pepper response: {e}")) + })?; Pepper::from_hex(&payload.pepper) }) } @@ -329,7 +369,12 @@ impl ProverService for HttpProverService { .await? .error_for_status()?; - let payload: ProverResponse = response.json().await?; + // SECURITY: Stream body with size limit to prevent OOM + let bytes = + crate::config::read_response_bounded(response, MAX_JWKS_RESPONSE_SIZE).await?; + let payload: ProverResponse = serde_json::from_slice(&bytes).map_err(|e| { + AptosError::InvalidJwt(format!("failed to parse prover response: {e}")) + })?; ZkProof::from_hex(&payload.proof) }) } @@ -715,6 +760,9 @@ impl AudClaim { /// Default timeout for JWKS fetch requests (10 seconds). const JWKS_FETCH_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); +/// Maximum JWKS response size: 1 MB (JWKS payloads are typically under 10 KB). +const MAX_JWKS_RESPONSE_SIZE: usize = 1024 * 1024; + /// Fetches the JWKS (JSON Web Key Set) from an OIDC provider. /// /// # Errors @@ -725,6 +773,18 @@ const JWKS_FETCH_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(1 /// - The JWKS endpoint returns a non-success status code /// - The response cannot be parsed as valid JWKS JSON async fn fetch_jwks(client: &reqwest::Client, jwks_url: &str) -> AptosResult { + // SECURITY: Validate the JWKS URL scheme to prevent SSRF. + // The issuer comes from an untrusted JWT, so the derived JWKS URL could + // point to internal services (e.g., cloud metadata endpoints). + let parsed_url = Url::parse(jwks_url) + .map_err(|e| AptosError::InvalidJwt(format!("invalid JWKS URL: {e}")))?; + if parsed_url.scheme() != "https" { + return Err(AptosError::InvalidJwt(format!( + "JWKS URL must use HTTPS scheme, got: {}", + parsed_url.scheme() + ))); + } + // Note: timeout is configured on the client, not per-request let response = client.get(jwks_url).send().await?; @@ -735,7 +795,11 @@ async fn fetch_jwks(client: &reqwest::Client, jwks_url: &str) -> AptosResult [u8; 64] { + /// + /// # Errors + /// + /// Returns an error if the mnemonic cannot be re-parsed (should not happen + /// since the phrase was validated during construction). + pub fn to_seed(&self) -> AptosResult<[u8; 64]> { self.to_seed_with_passphrase("") } /// Derives the seed from this mnemonic with a passphrase. /// - /// # Panics + /// # Errors /// - /// This function will never panic in normal operation because the mnemonic - /// phrase is validated during construction (`from_phrase` or `generate`). - /// The internal `expect` is a defensive check that should be unreachable. - pub fn to_seed_with_passphrase(&self, passphrase: &str) -> [u8; 64] { - // SAFETY: The mnemonic phrase was validated during construction. - // This expect should never trigger in normal operation. - let mnemonic = bip39::Mnemonic::parse_normalized(&self.phrase) - .expect("internal error: mnemonic was validated during construction"); - - mnemonic.to_seed(passphrase) + /// Returns an error if the mnemonic phrase cannot be re-parsed. This should + /// never happen because the phrase is validated during construction, but + /// returning an error is safer than panicking. + pub fn to_seed_with_passphrase(&self, passphrase: &str) -> AptosResult<[u8; 64]> { + let mnemonic = bip39::Mnemonic::parse_normalized(&self.phrase).map_err(|e| { + AptosError::InvalidMnemonic(format!("internal error: mnemonic re-parse failed: {e}")) + })?; + + Ok(mnemonic.to_seed(passphrase)) } /// Derives an Ed25519 private key using the Aptos derivation path. @@ -111,9 +121,15 @@ impl Mnemonic { /// Returns an error if key derivation fails or the derived key is invalid. #[cfg(feature = "ed25519")] pub fn derive_ed25519_key(&self, index: u32) -> AptosResult { - let seed = self.to_seed(); - let key = derive_ed25519_from_seed(&seed, index)?; - crate::crypto::Ed25519PrivateKey::from_bytes(&key) + let mut seed = self.to_seed()?; + let result = derive_ed25519_from_seed(&seed, index); + // SECURITY: Zeroize seed after use + zeroize::Zeroize::zeroize(&mut seed); + let mut key = result?; + let private_key = crate::crypto::Ed25519PrivateKey::from_bytes(&key); + // SECURITY: Zeroize raw key bytes after creating the key object + zeroize::Zeroize::zeroize(&mut key); + private_key } } @@ -160,8 +176,14 @@ fn derive_ed25519_from_seed(seed: &[u8], index: u32) -> AptosResult<[u8; 32]> { key.copy_from_slice(&result[..32]); chain_code.copy_from_slice(&result[32..]); + + // SECURITY: Zeroize intermediate derivation data + zeroize::Zeroize::zeroize(&mut data); } + // SECURITY: Zeroize chain_code since we only return the key + zeroize::Zeroize::zeroize(&mut chain_code); + Ok(key) } diff --git a/crates/aptos-sdk/src/api/faucet.rs b/crates/aptos-sdk/src/api/faucet.rs index 21d7dd5..a99dc6d 100644 --- a/crates/aptos-sdk/src/api/faucet.rs +++ b/crates/aptos-sdk/src/api/faucet.rs @@ -9,6 +9,9 @@ use serde::Deserialize; use std::sync::Arc; use url::Url; +/// Maximum faucet response size: 1 MB (faucet responses are typically tiny). +const MAX_FAUCET_RESPONSE_SIZE: usize = 1024 * 1024; + /// Client for the Aptos faucet service. /// /// The faucet is only available on devnet and testnet. Requests are @@ -103,6 +106,8 @@ impl FaucetClient { /// Returns an error if the URL cannot be parsed. pub fn with_url(url: &str) -> AptosResult { let faucet_url = Url::parse(url)?; + // SECURITY: Validate URL scheme to prevent SSRF via dangerous protocols + crate::config::validate_url_scheme(&faucet_url)?; let client = Client::new(); Ok(Self { faucet_url, @@ -132,7 +137,7 @@ impl FaucetClient { let client = self.client.clone(); let retry_config = self.retry_config.clone(); - let executor = RetryExecutor::new((*retry_config).clone()); + let executor = RetryExecutor::from_shared(retry_config); executor .execute(|| { let client = client.clone(); @@ -141,7 +146,14 @@ impl FaucetClient { let response = client.post(url).send().await?; if response.status().is_success() { - let faucet_response: FaucetResponse = response.json().await?; + // SECURITY: Stream body with size limit to prevent OOM + // from malicious responses (including chunked encoding). + let bytes = crate::config::read_response_bounded( + response, + MAX_FAUCET_RESPONSE_SIZE, + ) + .await?; + let faucet_response: FaucetResponse = serde_json::from_slice(&bytes)?; Ok(faucet_response.into_hashes()) } else { let status = response.status(); diff --git a/crates/aptos-sdk/src/api/fullnode.rs b/crates/aptos-sdk/src/api/fullnode.rs index c3f352b..f91ca70 100644 --- a/crates/aptos-sdk/src/api/fullnode.rs +++ b/crates/aptos-sdk/src/api/fullnode.rs @@ -19,6 +19,13 @@ const BCS_VIEW_CONTENT_TYPE: &str = "application/x-bcs"; const JSON_CONTENT_TYPE: &str = "application/json"; /// Default timeout for waiting for a transaction to be committed. const DEFAULT_TRANSACTION_WAIT_TIMEOUT_SECS: u64 = 30; +/// Maximum size for error response bodies (8 KB). +/// +/// # Security +/// +/// This prevents memory exhaustion from malicious servers sending extremely +/// large error response bodies. +const MAX_ERROR_BODY_SIZE: usize = 8 * 1024; /// Client for the Aptos fullnode REST API. /// @@ -273,7 +280,7 @@ impl FullnodeClient { let retry_config = self.retry_config.clone(); let max_response_size = self.config.pool_config().max_response_size; - let executor = RetryExecutor::new((*retry_config).clone()); + let executor = RetryExecutor::from_shared(retry_config); executor .execute(|| { let client = client.clone(); @@ -404,7 +411,7 @@ impl FullnodeClient { let retry_config = self.retry_config.clone(); let max_response_size = self.config.pool_config().max_response_size; - let executor = RetryExecutor::new((*retry_config).clone()); + let executor = RetryExecutor::from_shared(retry_config); executor .execute(|| { let client = client.clone(); @@ -464,7 +471,7 @@ impl FullnodeClient { let retry_config = self.retry_config.clone(); let max_response_size = self.config.pool_config().max_response_size; - let executor = RetryExecutor::new((*retry_config).clone()); + let executor = RetryExecutor::from_shared(retry_config); executor .execute(|| { let client = client.clone(); @@ -514,8 +521,8 @@ impl FullnodeClient { ) -> AptosResult>> { let url = self.build_url("view"); - // Convert BCS args to hex strings for the JSON request body - // The Aptos API accepts hex-encoded BCS bytes in the arguments array + // Convert BCS args to hex strings for the JSON request body. + // The Aptos API accepts hex-encoded BCS bytes in the arguments array. let hex_args: Vec = args .iter() .map(|bytes| serde_json::json!(const_hex::encode_prefixed(bytes))) @@ -529,8 +536,9 @@ impl FullnodeClient { let client = self.client.clone(); let retry_config = self.retry_config.clone(); + let max_response_size = self.config.pool_config().max_response_size; - let executor = RetryExecutor::new((*retry_config).clone()); + let executor = RetryExecutor::from_shared(retry_config); executor .execute(|| { let client = client.clone(); @@ -548,18 +556,28 @@ impl FullnodeClient { // Check for errors before reading body let status = response.status(); if !status.is_success() { - let error_text = response.text().await.unwrap_or_default(); + // SECURITY: Bound error body reads to prevent OOM from + // malicious servers sending huge error responses. + let error_bytes = + crate::config::read_response_bounded(response, MAX_ERROR_BODY_SIZE) + .await + .ok(); + let error_text = error_bytes + .and_then(|b| String::from_utf8(b).ok()) + .unwrap_or_default(); return Err(AptosError::Api { status_code: status.as_u16(), - message: error_text, + message: Self::truncate_error_body(error_text), error_code: None, vm_error_code: None, }); } - // Read the raw BCS bytes - let bytes = response.bytes().await?; - Ok(AptosResponse::new(bytes.to_vec())) + // SECURITY: Stream body with size limit to prevent OOM + // from malicious responses (including chunked encoding). + let bytes = + crate::config::read_response_bounded(response, max_response_size).await?; + Ok(AptosResponse::new(bytes)) } }) .await @@ -666,7 +684,7 @@ impl FullnodeClient { let retry_config = self.retry_config.clone(); let max_response_size = self.config.pool_config().max_response_size; - let executor = RetryExecutor::new((*retry_config).clone()); + let executor = RetryExecutor::from_shared(retry_config); executor .execute(|| { let client = client.clone(); @@ -684,33 +702,41 @@ impl FullnodeClient { .await } + /// Truncates a string to the maximum error body size. + /// + /// # Security + /// + /// Prevents storing extremely large error messages from malicious servers. + fn truncate_error_body(body: String) -> String { + if body.len() > MAX_ERROR_BODY_SIZE { + // Find the last valid UTF-8 char boundary at or before the limit + let mut end = MAX_ERROR_BODY_SIZE; + while end > 0 && !body.is_char_boundary(end) { + end -= 1; + } + format!( + "{}... [truncated, total: {} bytes]", + &body[..end], + body.len() + ) + } else { + body + } + } + /// Handles an HTTP response without retry (for internal use). /// /// # Security /// - /// This method checks the Content-Length header against `max_response_size` - /// to prevent memory exhaustion from extremely large responses. + /// This method enforces `max_response_size` on the actual response body, + /// not just the Content-Length header, to prevent memory exhaustion even + /// when the server uses chunked transfer encoding. async fn handle_response_static serde::Deserialize<'de>>( response: reqwest::Response, max_response_size: usize, ) -> AptosResult> { let status = response.status(); - // SECURITY: Check Content-Length to prevent memory exhaustion - // This protects against malicious servers sending extremely large responses - if let Some(content_length) = response.content_length() - && content_length > max_response_size as u64 - { - return Err(AptosError::Api { - status_code: status.as_u16(), - message: format!( - "response body too large: {content_length} bytes exceeds limit of {max_response_size} bytes" - ), - error_code: Some("RESPONSE_TOO_LARGE".to_string()), - vm_error_code: None, - }); - } - // Extract headers before consuming response body let ledger_version = response .headers() @@ -751,7 +777,10 @@ impl FullnodeClient { .and_then(|v| v.parse().ok()); if status.is_success() { - let data: T = response.json().await?; + // SECURITY: Stream body with size limit to prevent OOM + // from malicious responses (including chunked encoding). + let bytes = crate::config::read_response_bounded(response, max_response_size).await?; + let data: T = serde_json::from_slice(&bytes)?; Ok(AptosResponse { data, ledger_version, @@ -766,7 +795,16 @@ impl FullnodeClient { // This allows callers to respect the server's rate limiting Err(AptosError::RateLimited { retry_after_secs }) } else { - let body: serde_json::Value = response.json().await.unwrap_or_default(); + // SECURITY: Bound error body reads to prevent OOM from malicious + // servers sending huge error responses (including chunked encoding). + let error_bytes = crate::config::read_response_bounded(response, MAX_ERROR_BODY_SIZE) + .await + .ok(); + let error_text = error_bytes + .and_then(|b| String::from_utf8(b).ok()) + .unwrap_or_default(); + let error_text = Self::truncate_error_body(error_text); + let body: serde_json::Value = serde_json::from_str(&error_text).unwrap_or_default(); let message = body .get("message") .and_then(|v| v.as_str()) diff --git a/crates/aptos-sdk/src/api/indexer.rs b/crates/aptos-sdk/src/api/indexer.rs index b8b8e97..603b8ba 100644 --- a/crates/aptos-sdk/src/api/indexer.rs +++ b/crates/aptos-sdk/src/api/indexer.rs @@ -38,6 +38,9 @@ use serde::{Deserialize, Serialize}; use std::sync::Arc; use url::Url; +/// Maximum indexer response size: 10 MB. +const MAX_INDEXER_RESPONSE_SIZE: usize = 10 * 1024 * 1024; + /// Client for the Aptos indexer GraphQL API. /// /// The indexer provides access to indexed blockchain data including @@ -136,6 +139,8 @@ impl IndexerClient { /// Returns an error if the URL cannot be parsed. pub fn with_url(url: &str) -> AptosResult { let indexer_url = Url::parse(url)?; + // SECURITY: Validate URL scheme to prevent SSRF via dangerous protocols + crate::config::validate_url_scheme(&indexer_url)?; let client = Client::new(); Ok(Self { indexer_url, @@ -165,7 +170,7 @@ impl IndexerClient { let url = self.indexer_url.clone(); let retry_config = self.retry_config.clone(); - let executor = RetryExecutor::new((*retry_config).clone()); + let executor = RetryExecutor::from_shared(retry_config); executor .execute(|| { let client = client.clone(); @@ -178,15 +183,28 @@ impl IndexerClient { let response = client.post(url.as_str()).json(&request).send().await?; if response.status().is_success() { - let graphql_response: GraphQLResponse = response.json().await?; + // SECURITY: Stream body with size limit to prevent OOM + // from malicious responses (including chunked encoding). + let bytes = crate::config::read_response_bounded( + response, + MAX_INDEXER_RESPONSE_SIZE, + ) + .await?; + let graphql_response: GraphQLResponse = serde_json::from_slice(&bytes)?; if let Some(errors) = graphql_response.errors { - let messages: Vec = - errors.iter().map(|e| e.message.clone()).collect(); + // Build error message directly without intermediate Vec + let mut message = String::new(); + for (i, e) in errors.iter().enumerate() { + if i > 0 { + message.push_str("; "); + } + message.push_str(&e.message); + } return Err(AptosError::Api { status_code: 400, - message: messages.join("; "), - error_code: Some("GRAPHQL_ERROR".to_string()), + message, + error_code: Some("GRAPHQL_ERROR".into()), vm_error_code: None, }); } diff --git a/crates/aptos-sdk/src/aptos.rs b/crates/aptos-sdk/src/aptos.rs index 7d79680..64073e4 100644 --- a/crates/aptos-sdk/src/aptos.rs +++ b/crates/aptos-sdk/src/aptos.rs @@ -10,7 +10,8 @@ use crate::transaction::{ RawTransaction, SignedTransaction, TransactionBuilder, TransactionPayload, }; use crate::types::{AccountAddress, ChainId}; -use std::sync::{Arc, RwLock}; +use std::sync::Arc; +use std::sync::atomic::{AtomicU8, Ordering}; use std::time::Duration; #[cfg(feature = "ed25519")] @@ -54,7 +55,8 @@ pub struct Aptos { fullnode: Arc, /// Resolved chain ID. Initialized from config; lazily fetched from node /// for custom networks where the chain ID is unknown (0). - chain_id: RwLock, + /// Stored as `AtomicU8` to avoid lock overhead for this single-byte value. + chain_id: AtomicU8, #[cfg(feature = "faucet")] faucet: Option, #[cfg(feature = "indexer")] @@ -76,7 +78,7 @@ impl Aptos { #[cfg(feature = "indexer")] let indexer = IndexerClient::new(&config).ok(); - let chain_id = RwLock::new(config.chain_id()); + let chain_id = AtomicU8::new(config.chain_id().id()); Ok(Self { config, @@ -158,18 +160,17 @@ impl Aptos { /// /// Returns an error if the HTTP request fails, the API returns an error status code, /// or the response cannot be parsed. - /// - /// # Panics - /// - /// Panics if the internal `chain_id` lock is poisoned (only possible if another thread - /// panicked while holding the lock). pub async fn ledger_info(&self) -> AptosResult { let response = self.fullnode.get_ledger_info().await?; let info = response.into_inner(); - // Update chain_id if it was unknown (custom network) - if self.chain_id.read().expect("chain_id lock poisoned").id() == 0 && info.chain_id > 0 { - *self.chain_id.write().expect("chain_id lock poisoned") = ChainId::new(info.chain_id); + // Update chain_id if it was unknown (custom network). + // NOTE: The load-then-store pattern has a benign TOCTOU race: multiple + // threads may concurrently see chain_id == 0 and all store the same + // value from the ledger info response. This is safe because they always + // store the identical chain_id value returned by the node. + if self.chain_id.load(Ordering::Relaxed) == 0 && info.chain_id > 0 { + self.chain_id.store(info.chain_id, Ordering::Relaxed); } Ok(info) @@ -183,11 +184,8 @@ impl Aptos { /// or any method that makes a request to the node (e.g., [`build_transaction`](Self::build_transaction), /// [`ledger_info`](Self::ledger_info)). /// - /// # Panics - /// - /// Panics if the internal `chain_id` lock is poisoned. pub fn chain_id(&self) -> ChainId { - *self.chain_id.read().expect("chain_id lock poisoned") + ChainId::new(self.chain_id.load(Ordering::Relaxed)) } /// Resolves the chain ID from the node if it is unknown. @@ -205,22 +203,16 @@ impl Aptos { /// /// Returns an error if the HTTP request to fetch ledger info fails. /// - /// # Panics - /// - /// Panics if the internal `chain_id` lock is poisoned. pub async fn ensure_chain_id(&self) -> AptosResult { - { - let chain_id = self.chain_id.read().expect("chain_id lock poisoned"); - if chain_id.id() > 0 { - return Ok(*chain_id); - } + let id = self.chain_id.load(Ordering::Relaxed); + if id > 0 { + return Ok(ChainId::new(id)); } // Chain ID is unknown; fetch from node let response = self.fullnode.get_ledger_info().await?; let info = response.into_inner(); - let new_chain_id = ChainId::new(info.chain_id); - *self.chain_id.write().expect("chain_id lock poisoned") = new_chain_id; - Ok(new_chain_id) + self.chain_id.store(info.chain_id, Ordering::Relaxed); + Ok(ChainId::new(info.chain_id)) } // === Account === @@ -477,7 +469,7 @@ impl Aptos { payload: TransactionPayload, ) -> AptosResult> { // First simulate - let raw_txn = self.build_transaction(account, payload.clone()).await?; + let raw_txn = self.build_transaction(account, payload).await?; let signed = crate::transaction::builder::sign_transaction(&raw_txn, account)?; let sim_response = self.fullnode.simulate_transaction(&signed).await?; let sim_result = @@ -513,7 +505,7 @@ impl Aptos { timeout: Option, ) -> AptosResult> { // First simulate - let raw_txn = self.build_transaction(account, payload.clone()).await?; + let raw_txn = self.build_transaction(account, payload).await?; let signed = crate::transaction::builder::sign_transaction(&raw_txn, account)?; let sim_response = self.fullnode.simulate_transaction(&signed).await?; let sim_result = diff --git a/crates/aptos-sdk/src/codegen/build_helper.rs b/crates/aptos-sdk/src/codegen/build_helper.rs index c3f87dd..7e7d4e1 100644 --- a/crates/aptos-sdk/src/codegen/build_helper.rs +++ b/crates/aptos-sdk/src/codegen/build_helper.rs @@ -48,6 +48,89 @@ use crate::error::{AptosError, AptosResult}; use std::fs; use std::path::Path; +/// Returns true if `name` is a Rust keyword that cannot be used as a module name. +fn is_rust_keyword(name: &str) -> bool { + matches!( + name, + "as" | "break" + | "const" + | "continue" + | "crate" + | "else" + | "enum" + | "extern" + | "false" + | "fn" + | "for" + | "if" + | "impl" + | "in" + | "let" + | "loop" + | "match" + | "mod" + | "move" + | "mut" + | "pub" + | "ref" + | "return" + | "self" + | "Self" + | "static" + | "struct" + | "super" + | "trait" + | "true" + | "type" + | "unsafe" + | "use" + | "where" + | "while" + | "async" + | "await" + | "dyn" + ) +} + +/// Validates that a module name is a safe Rust identifier (no path traversal, injection, or keywords). +/// +/// # Security +/// +/// This prevents: +/// - Path traversal attacks via names like `../../../tmp/evil` +/// - Invalid `pub mod` declarations in generated mod.rs (e.g., `pub mod fn;`) +fn validate_module_name(name: &str) -> AptosResult<()> { + if name.is_empty() { + return Err(AptosError::Config( + "module name cannot be empty".to_string(), + )); + } + + // Must be a valid Rust identifier: starts with letter or underscore, + // contains only alphanumeric or underscore characters + let mut chars = name.chars(); + let first = chars.next().unwrap(); // safe: name is non-empty + if !first.is_ascii_alphabetic() && first != '_' { + return Err(AptosError::Config(format!( + "invalid module name '{name}': must start with a letter or underscore" + ))); + } + + if !chars.all(|c| c.is_ascii_alphanumeric() || c == '_') { + return Err(AptosError::Config(format!( + "invalid module name '{name}': must contain only ASCII alphanumeric characters or underscores" + ))); + } + + if is_rust_keyword(name) { + return Err(AptosError::Config(format!( + "invalid module name '{name}': Rust keywords cannot be used as module names" + ))); + } + + Ok(()) +} + /// Configuration for build-time code generation. #[derive(Debug, Clone)] pub struct BuildConfig { @@ -156,6 +239,9 @@ pub fn generate_from_abi_with_config( let abi: MoveModuleABI = serde_json::from_str(&abi_content) .map_err(|e| AptosError::Config(format!("Failed to parse ABI JSON: {e}")))?; + // SECURITY: Validate module name to prevent path traversal + validate_module_name(&abi.name)?; + // Generate code let generator = ModuleGenerator::new(&abi, config.generator_config); let code = generator.generate()?; @@ -251,6 +337,9 @@ pub fn generate_from_abis_with_config( )) })?; + // SECURITY: Validate module name to prevent path traversal + validate_module_name(&abi.name)?; + let generator = ModuleGenerator::new(&abi, config.generator_config.clone()); let code = generator.generate()?; @@ -323,6 +412,9 @@ pub fn generate_from_abi_with_source( let source_info = MoveSourceParser::parse(&source_content); + // SECURITY: Validate module name to prevent path traversal + validate_module_name(&abi.name)?; + // Generate code let generator = ModuleGenerator::new(&abi, GeneratorConfig::default()).with_source_info(source_info); @@ -359,6 +451,12 @@ fn generate_mod_file(module_names: &[String]) -> String { let _ = writeln!(&mut content); for name in module_names { + // SECURITY: Module names are validated by validate_module_name() before reaching here, + // but double-check they are safe identifiers to prevent code injection in mod.rs + debug_assert!( + !name.is_empty() && name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_'), + "module name should have been validated" + ); let _ = writeln!(&mut content, "pub mod {name};"); } let _ = writeln!(&mut content); diff --git a/crates/aptos-sdk/src/codegen/generator.rs b/crates/aptos-sdk/src/codegen/generator.rs index 4a6fd6b..6112881 100644 --- a/crates/aptos-sdk/src/codegen/generator.rs +++ b/crates/aptos-sdk/src/codegen/generator.rs @@ -6,6 +6,20 @@ use crate::codegen::types::{MoveTypeMapper, to_pascal_case, to_snake_case}; use crate::error::{AptosError, AptosResult}; use std::fmt::Write; +/// Sanitizes an ABI-derived string for safe embedding in generated Rust code. +/// +/// # Security +/// +/// Prevents code injection via crafted ABI JSON by: +/// - Replacing newlines (which could escape doc comments and inject code) +/// - Escaping double quotes (which could break string literals) +/// - Escaping backslashes (which could create escape sequences) +fn sanitize_abi_string(s: &str) -> String { + s.replace('\\', "\\\\") + .replace('"', "\\\"") + .replace(['\n', '\r'], " ") +} + /// Configuration for code generation. #[derive(Debug, Clone)] #[allow(clippy::struct_excessive_bools)] // Config structs often have boolean options @@ -234,7 +248,10 @@ impl<'a> ModuleGenerator<'a> { writeln!( output, " pub const {}: &str = \"{}::{}::{}\";", - const_name, self.abi.address, self.abi.name, struct_def.name + const_name, + sanitize_abi_string(&self.abi.address), + sanitize_abi_string(&self.abi.name), + sanitize_abi_string(&struct_def.name) )?; } writeln!(output, "}}")?; @@ -285,7 +302,8 @@ impl<'a> ModuleGenerator<'a> { writeln!( output, " event_type.starts_with(\"{}::{}::\")", - self.abi.address, self.abi.name + sanitize_abi_string(&self.abi.address), + sanitize_abi_string(&self.abi.name) )?; writeln!(output, "}}")?; writeln!(output) @@ -296,7 +314,8 @@ impl<'a> ModuleGenerator<'a> { writeln!( output, "//! Generated Rust bindings for `{}::{}`.", - self.abi.address, self.abi.name + sanitize_abi_string(&self.abi.address), + sanitize_abi_string(&self.abi.name) )?; writeln!(output, "//!")?; writeln!( @@ -333,14 +352,14 @@ impl<'a> ModuleGenerator<'a> { writeln!( output, "pub const MODULE_ADDRESS: &str = \"{}\";", - self.abi.address + sanitize_abi_string(&self.abi.address) )?; writeln!(output)?; writeln!(output, "/// The module name.")?; writeln!( output, "pub const MODULE_NAME: &str = \"{}\";", - self.abi.name + sanitize_abi_string(&self.abi.name) )?; writeln!(output) } @@ -373,14 +392,22 @@ impl<'a> ModuleGenerator<'a> { let rust_name = to_pascal_case(&struct_def.name); // Documentation + // SECURITY: Sanitize ABI-derived strings to prevent code injection via newlines writeln!( output, "/// Move struct: `{}::{}`", - self.abi.name, struct_def.name + sanitize_abi_string(&self.abi.name), + sanitize_abi_string(&struct_def.name) )?; if !struct_def.abilities.is_empty() { + let abilities = struct_def + .abilities + .iter() + .map(|a| sanitize_abi_string(a)) + .collect::>() + .join(", "); writeln!(output, "///")?; - writeln!(output, "/// Abilities: {}", struct_def.abilities.join(", "))?; + writeln!(output, "/// Abilities: {abilities}")?; } // Derive macros @@ -431,12 +458,18 @@ impl<'a> ModuleGenerator<'a> { }; if let Some(doc) = &rust_type.doc { - writeln!(output, " /// {doc}")?; + // SECURITY: Sanitize doc strings derived from ABI to prevent code injection + writeln!(output, " /// {}", sanitize_abi_string(doc))?; } // Add serde rename if field name differs if rust_name != field.name && !rust_name.starts_with("r#") { - writeln!(output, " #[serde(rename = \"{}\")]", field.name)?; + // SECURITY: Escape field names to prevent breaking the serde attribute syntax + writeln!( + output, + " #[serde(rename = \"{}\")]", + sanitize_abi_string(&field.name) + )?; } writeln!(output, " pub {}: {},", rust_name, rust_type.path) @@ -483,16 +516,18 @@ impl<'a> ModuleGenerator<'a> { .collect(); // Documentation from source or generated + // SECURITY: Sanitize ABI-derived strings to prevent code injection via newlines if let Some(doc) = &enriched.doc { for line in doc.lines() { - writeln!(output, "/// {line}")?; + writeln!(output, "/// {}", sanitize_abi_string(line))?; } writeln!(output, "///")?; } else { writeln!( output, "/// Entry function: `{}::{}`", - self.abi.name, func.name + sanitize_abi_string(&self.abi.name), + sanitize_abi_string(&func.name) )?; writeln!(output, "///")?; } @@ -505,16 +540,20 @@ impl<'a> ModuleGenerator<'a> { writeln!( output, "/// * `{}` - {} (Move type: `{}`)", - name, rust_type.path, move_type + sanitize_abi_string(name), + sanitize_abi_string(&rust_type.path), + sanitize_abi_string(move_type) )?; } } if !enriched.type_param_names.is_empty() { - writeln!( - output, - "/// * `type_args` - Type arguments: {}", - enriched.type_param_names.join(", ") - )?; + let type_params = enriched + .type_param_names + .iter() + .map(|s| sanitize_abi_string(s)) + .collect::>() + .join(", "); + writeln!(output, "/// * `type_args` - Type arguments: {type_params}")?; } // Function signature @@ -535,11 +574,14 @@ impl<'a> ModuleGenerator<'a> { writeln!(output, ") -> AptosResult {{")?; // Function body + // SECURITY: Sanitize ABI-derived values embedded in string literals writeln!(output, " let function_id = format!(")?; writeln!( output, " \"{}::{}::{}\",", - self.abi.address, self.abi.name, func.name + sanitize_abi_string(&self.abi.address), + sanitize_abi_string(&self.abi.name), + sanitize_abi_string(&func.name) )?; writeln!(output, " );")?; writeln!(output)?; @@ -673,16 +715,18 @@ impl<'a> ModuleGenerator<'a> { }; // Documentation from source or generated + // SECURITY: Sanitize ABI-derived strings to prevent code injection via newlines if let Some(doc) = &enriched.doc { for line in doc.lines() { - writeln!(output, "/// {line}")?; + writeln!(output, "/// {}", sanitize_abi_string(line))?; } writeln!(output, "///")?; } else { writeln!( output, "/// View function: `{}::{}`", - self.abi.name, func.name + sanitize_abi_string(&self.abi.name), + sanitize_abi_string(&func.name) )?; writeln!(output, "///")?; } @@ -695,22 +739,26 @@ impl<'a> ModuleGenerator<'a> { writeln!( output, "/// * `{}` - {} (Move type: `{}`)", - name, rust_type.path, move_type + sanitize_abi_string(name), + sanitize_abi_string(&rust_type.path), + sanitize_abi_string(move_type) )?; } } if !enriched.type_param_names.is_empty() { - writeln!( - output, - "/// * `type_args` - Type arguments: {}", - enriched.type_param_names.join(", ") - )?; + let type_params = enriched + .type_param_names + .iter() + .map(|s| sanitize_abi_string(s)) + .collect::>() + .join(", "); + writeln!(output, "/// * `type_args` - Type arguments: {type_params}")?; } if !func.returns.is_empty() { writeln!(output, "///")?; writeln!(output, "/// # Returns")?; writeln!(output, "///")?; - writeln!(output, "/// `{return_type}`")?; + writeln!(output, "/// `{}`", sanitize_abi_string(&return_type))?; } // Function signature @@ -734,11 +782,14 @@ impl<'a> ModuleGenerator<'a> { writeln!(output, ") -> AptosResult> {{")?; // Function body + // SECURITY: Sanitize ABI-derived values embedded in string literals writeln!(output, " let function_id = format!(")?; writeln!( output, " \"{}::{}::{}\",", - self.abi.address, self.abi.name, func.name + sanitize_abi_string(&self.abi.address), + sanitize_abi_string(&self.abi.name), + sanitize_abi_string(&func.name) )?; writeln!(output, " );")?; writeln!(output)?; diff --git a/crates/aptos-sdk/src/codegen/move_parser.rs b/crates/aptos-sdk/src/codegen/move_parser.rs index 3311deb..1a84c46 100644 --- a/crates/aptos-sdk/src/codegen/move_parser.rs +++ b/crates/aptos-sdk/src/codegen/move_parser.rs @@ -48,9 +48,24 @@ pub struct MoveModuleInfo { #[derive(Debug, Clone, Copy, Default)] pub struct MoveSourceParser; +/// Maximum Move source file size for parsing (10 MB). +/// +/// # Security +/// +/// Prevents excessive memory usage when parsing very large or malicious input. +const MAX_SOURCE_SIZE: usize = 10 * 1024 * 1024; + impl MoveSourceParser { /// Parses Move source code and extracts module information. + /// + /// # Security + /// + /// Returns an empty `MoveModuleInfo` if the source exceeds `MAX_SOURCE_SIZE` + /// (10 MB) to prevent memory exhaustion from extremely large inputs. pub fn parse(source: &str) -> MoveModuleInfo { + if source.len() > MAX_SOURCE_SIZE { + return MoveModuleInfo::default(); + } MoveModuleInfo { doc: Self::extract_leading_doc(source), functions: Self::parse_functions(source), @@ -126,7 +141,7 @@ impl MoveSourceParser { let mut info = MoveFunctionInfo::default(); let mut consumed = 0; - // Look backwards for doc comments + // Look backwards for doc comments, collecting in reverse then reversing let mut doc_lines = Vec::new(); let mut j = start; while j > 0 { @@ -134,7 +149,7 @@ impl MoveSourceParser { let prev_line = lines[j].trim(); if prev_line.starts_with("///") { let doc_content = prev_line.strip_prefix("///").unwrap_or("").trim(); - doc_lines.insert(0, doc_content.to_string()); + doc_lines.push(doc_content.to_string()); } else if prev_line.is_empty() || prev_line.starts_with("#[") { // Skip empty lines and attributes } else { @@ -143,6 +158,7 @@ impl MoveSourceParser { } if !doc_lines.is_empty() { + doc_lines.reverse(); info.doc = Some(doc_lines.join("\n")); } @@ -350,7 +366,7 @@ impl MoveSourceParser { let mut info = MoveStructInfo::default(); let mut consumed = 0; - // Look backwards for doc comments + // Look backwards for doc comments, collecting in reverse then reversing let mut doc_lines = Vec::new(); let mut j = start; while j > 0 { @@ -358,7 +374,7 @@ impl MoveSourceParser { let prev_line = lines[j].trim(); if prev_line.starts_with("///") { let doc_content = prev_line.strip_prefix("///").unwrap_or("").trim(); - doc_lines.insert(0, doc_content.to_string()); + doc_lines.push(doc_content.to_string()); } else if prev_line.is_empty() || prev_line.starts_with("#[") { // Skip empty lines and attributes } else { @@ -367,6 +383,7 @@ impl MoveSourceParser { } if !doc_lines.is_empty() { + doc_lines.reverse(); info.doc = Some(doc_lines.join("\n")); } diff --git a/crates/aptos-sdk/src/codegen/types.rs b/crates/aptos-sdk/src/codegen/types.rs index 10a1cb9..aa2238c 100644 --- a/crates/aptos-sdk/src/codegen/types.rs +++ b/crates/aptos-sdk/src/codegen/types.rs @@ -179,23 +179,14 @@ impl MoveTypeMapper { if !rust_type.needs_bcs { // Primitives that don't need special handling - return format!("aptos_bcs::to_bytes(&{var_name}).unwrap()"); + return format!( + "aptos_bcs::to_bytes(&{var_name}).map_err(|e| AptosError::Bcs(e.to_string()))?" + ); } - match move_type { - "address" => format!("aptos_bcs::to_bytes(&{var_name}).unwrap()"), - _ if move_type.starts_with("vector") => { - format!("aptos_bcs::to_bytes(&{var_name}).unwrap()") - } - _ if move_type.starts_with("vector<") => { - format!("aptos_bcs::to_bytes(&{var_name}).unwrap()") - } - "0x1::string::String" => format!("aptos_bcs::to_bytes(&{var_name}).unwrap()"), - _ if move_type.ends_with("::string::String") => { - format!("aptos_bcs::to_bytes(&{var_name}).unwrap()") - } - _ => format!("aptos_bcs::to_bytes(&{var_name}).unwrap()"), - } + // SECURITY: Use error propagation instead of .unwrap() to prevent + // panics in generated code if BCS serialization fails. + format!("aptos_bcs::to_bytes(&{var_name}).map_err(|e| AptosError::Bcs(e.to_string()))?") } /// Determines if a parameter should be excluded from the function signature. diff --git a/crates/aptos-sdk/src/config.rs b/crates/aptos-sdk/src/config.rs index c82bf44..ab3533c 100644 --- a/crates/aptos-sdk/src/config.rs +++ b/crates/aptos-sdk/src/config.rs @@ -14,14 +14,17 @@ use url::Url; /// # Security /// /// This prevents SSRF attacks via dangerous URL schemes like `file://`, `gopher://`, etc. -/// For production use, HTTPS is strongly recommended. HTTP is allowed for localhost -/// development only. -fn validate_url_scheme(url: &Url) -> AptosResult<()> { +/// For production use, HTTPS is strongly recommended. HTTP is permitted (e.g., for local +/// development) but no host restrictions are enforced by this function. +/// +/// # Errors +/// +/// Returns [`AptosError::Config`] if the URL scheme is not `http` or `https`. +pub fn validate_url_scheme(url: &Url) -> AptosResult<()> { match url.scheme() { "https" => Ok(()), "http" => { - // HTTP is allowed but only recommended for localhost development - // Log a warning in the future if we add logging + // HTTP is allowed for local development and testing Ok(()) } scheme => Err(AptosError::Config(format!( @@ -30,6 +33,60 @@ fn validate_url_scheme(url: &Url) -> AptosResult<()> { } } +/// Reads a response body with an enforced size limit, aborting early if exceeded. +/// +/// Unlike `response.bytes().await?` which buffers the entire response in memory +/// before any size check, this function: +/// 1. Pre-checks the `Content-Length` header (if present) to reject obviously +/// oversized responses before reading any body data. +/// 2. Reads the body incrementally via chunked streaming, aborting as soon as +/// the accumulated size exceeds `max_size`. +/// +/// This prevents memory exhaustion from malicious servers that send huge +/// responses (including chunked transfer-encoding without `Content-Length`). +/// +/// # Errors +/// +/// Returns [`AptosError::Api`] with error code `RESPONSE_TOO_LARGE` if the +/// response body exceeds `max_size` bytes. +pub async fn read_response_bounded( + mut response: reqwest::Response, + max_size: usize, +) -> AptosResult> { + // Pre-check Content-Length header for early rejection (avoids reading any body) + if let Some(content_length) = response.content_length() + && content_length > max_size as u64 + { + return Err(AptosError::Api { + status_code: response.status().as_u16(), + message: format!( + "response too large: Content-Length {content_length} bytes exceeds limit of {max_size} bytes" + ), + error_code: Some("RESPONSE_TOO_LARGE".into()), + vm_error_code: None, + }); + } + + // Read body incrementally, aborting if accumulated size exceeds the limit. + // This protects against chunked transfer-encoding that bypasses Content-Length. + let mut body = Vec::with_capacity(std::cmp::min(max_size, 1024 * 1024)); + while let Some(chunk) = response.chunk().await? { + if body.len().saturating_add(chunk.len()) > max_size { + return Err(AptosError::Api { + status_code: response.status().as_u16(), + message: format!( + "response too large: exceeded limit of {max_size} bytes during streaming" + ), + error_code: Some("RESPONSE_TOO_LARGE".into()), + vm_error_code: None, + }); + } + body.extend_from_slice(&chunk); + } + + Ok(body) +} + /// Configuration for HTTP connection pooling. /// /// Controls how connections are reused across requests for better performance. @@ -51,7 +108,7 @@ pub struct PoolConfig { /// Default: true pub tcp_nodelay: bool, /// Maximum response body size in bytes. - /// Default: 100 MB (`104_857_600` bytes) + /// Default: 10 MB (`10_485_760` bytes) /// /// # Security /// @@ -60,8 +117,16 @@ pub struct PoolConfig { pub max_response_size: usize, } -/// Default maximum response size: 100 MB -const DEFAULT_MAX_RESPONSE_SIZE: usize = 100 * 1024 * 1024; +/// Default maximum response size: 10 MB +/// +/// # Security +/// +/// This limit helps prevent memory exhaustion from malicious or compromised +/// servers sending extremely large responses. The default of 10 MB is generous +/// for normal Aptos API responses (typically under 1 MB). If you need to +/// handle larger responses (e.g., bulk data exports), increase this via +/// [`PoolConfigBuilder::max_response_size`]. +const DEFAULT_MAX_RESPONSE_SIZE: usize = 10 * 1024 * 1024; impl Default for PoolConfig { fn default() -> Self { @@ -811,4 +876,83 @@ mod tests { assert!(config.pool_config().max_idle_total > 0); assert_eq!(config.chain_id().id(), 2); } + + #[tokio::test] + async fn test_read_response_bounded_normal() { + use wiremock::{Mock, MockServer, ResponseTemplate, matchers::method}; + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200).set_body_string("hello world")) + .mount(&server) + .await; + + let response = reqwest::get(server.uri()).await.unwrap(); + let body = read_response_bounded(response, 1024).await.unwrap(); + assert_eq!(body, b"hello world"); + } + + #[tokio::test] + async fn test_read_response_bounded_rejects_oversized_content_length() { + use wiremock::{Mock, MockServer, ResponseTemplate, matchers::method}; + let server = MockServer::start().await; + // Send a body whose accurate Content-Length exceeds the limit. + // The function should reject based on Content-Length pre-check + // before streaming the full body. + let body = "x".repeat(200); + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200).set_body_string(body)) + .mount(&server) + .await; + + let response = reqwest::get(server.uri()).await.unwrap(); + // Limit is 100 but body is 200 -- should be rejected via Content-Length pre-check + let result = read_response_bounded(response, 100).await; + assert!(result.is_err()); + let err = result.unwrap_err().to_string(); + assert!(err.contains("response too large")); + } + + #[tokio::test] + async fn test_read_response_bounded_rejects_oversized_body() { + use wiremock::{Mock, MockServer, ResponseTemplate, matchers::method}; + let server = MockServer::start().await; + let large_body = "x".repeat(500); + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200).set_body_string(large_body)) + .mount(&server) + .await; + + let response = reqwest::get(server.uri()).await.unwrap(); + let result = read_response_bounded(response, 100).await; + assert!(result.is_err()); + } + + #[tokio::test] + async fn test_read_response_bounded_exact_limit() { + use wiremock::{Mock, MockServer, ResponseTemplate, matchers::method}; + let server = MockServer::start().await; + let body = "x".repeat(100); + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200).set_body_string(body.clone())) + .mount(&server) + .await; + + let response = reqwest::get(server.uri()).await.unwrap(); + let result = read_response_bounded(response, 100).await.unwrap(); + assert_eq!(result.len(), 100); + } + + #[tokio::test] + async fn test_read_response_bounded_empty() { + use wiremock::{Mock, MockServer, ResponseTemplate, matchers::method}; + let server = MockServer::start().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + let response = reqwest::get(server.uri()).await.unwrap(); + let result = read_response_bounded(response, 1024).await.unwrap(); + assert!(result.is_empty()); + } } diff --git a/crates/aptos-sdk/src/crypto/ed25519.rs b/crates/aptos-sdk/src/crypto/ed25519.rs index 1ff94f8..279e79f 100644 --- a/crates/aptos-sdk/src/crypto/ed25519.rs +++ b/crates/aptos-sdk/src/crypto/ed25519.rs @@ -68,6 +68,8 @@ impl Ed25519PrivateKey { let mut key_bytes = [0u8; ED25519_PRIVATE_KEY_LENGTH]; key_bytes.copy_from_slice(bytes); let signing_key = ed25519_dalek::SigningKey::from_bytes(&key_bytes); + // SECURITY: Zeroize the temporary buffer that held private key material + zeroize::Zeroize::zeroize(&mut key_bytes); Ok(Self { inner: signing_key }) } diff --git a/crates/aptos-sdk/src/crypto/secp256k1.rs b/crates/aptos-sdk/src/crypto/secp256k1.rs index 527f1fe..697ae85 100644 --- a/crates/aptos-sdk/src/crypto/secp256k1.rs +++ b/crates/aptos-sdk/src/crypto/secp256k1.rs @@ -4,14 +4,15 @@ //! //! # Security: Signature Malleability //! -//! This implementation uses the `k256` crate which produces normalized (low-S) -//! ECDSA signatures by default. This prevents signature malleability attacks -//! where an attacker could create an alternative valid signature `(r, -s mod n)` -//! for the same message. +//! This implementation enforces **low-S only** signatures to match the Aptos +//! blockchain's on-chain verification, which rejects high-S signatures to +//! prevent signature malleability attacks. //! -//! When parsing external signatures via [`Secp256k1Signature::from_bytes`], the -//! `k256` crate accepts both low-S and high-S signatures. If strict low-S -//! enforcement is required, callers should normalize or reject high-S signatures. +//! - **Signing** always produces low-S signatures (the `k256` crate normalizes +//! by default). +//! - **Parsing** (`from_bytes`, `from_hex`) rejects high-S signatures. +//! - **Verification** also rejects high-S signatures as a defense-in-depth +//! measure. use crate::crypto::traits::{PublicKey, Signature, Signer, Verifier}; use crate::error::{AptosError, AptosResult}; @@ -124,18 +125,29 @@ impl Secp256k1PrivateKey { } } - /// Signs a message (pre-hashed with SHA256) and returns the signature. + /// Signs a message (pre-hashed with SHA256) and returns a low-S signature. + /// + /// The `k256` crate produces low-S signatures by default. An additional + /// normalization step is included as defense-in-depth to guarantee Aptos + /// on-chain compatibility. pub fn sign(&self, message: &[u8]) -> Secp256k1Signature { // Hash the message with SHA256 first (standard for ECDSA) let hash = crate::crypto::sha2_256(message); let signature: K256Signature = self.inner.sign(&hash); - Secp256k1Signature { inner: signature } + // SECURITY: Ensure low-S (defense-in-depth; k256 should already do this) + let normalized = signature.normalize_s().unwrap_or(signature); + Secp256k1Signature { inner: normalized } } - /// Signs a pre-hashed message directly. + /// Signs a pre-hashed message directly and returns a low-S signature. + /// + /// The `k256` crate produces low-S signatures by default. An additional + /// normalization step is included as defense-in-depth. pub fn sign_prehashed(&self, hash: &[u8; 32]) -> Secp256k1Signature { let signature: K256Signature = self.inner.sign(hash); - Secp256k1Signature { inner: signature } + // SECURITY: Ensure low-S (defense-in-depth; k256 should already do this) + let normalized = signature.normalize_s().unwrap_or(signature); + Secp256k1Signature { inner: normalized } } } @@ -232,11 +244,21 @@ impl Secp256k1PublicKey { /// Verifies a signature against a message. /// + /// # Security + /// + /// Rejects high-S signatures before verification, matching Aptos on-chain + /// behavior. This is a defense-in-depth check; signatures created through + /// this SDK's `from_bytes` are already guaranteed to be low-S. + /// /// # Errors /// - /// Returns [`AptosError::SignatureVerificationFailed`] if the signature is invalid or does not match the message. + /// Returns [`AptosError::SignatureVerificationFailed`] if the signature has + /// a high-S value, is invalid, or does not match the message. pub fn verify(&self, message: &[u8], signature: &Secp256k1Signature) -> AptosResult<()> { - // Hash the message with SHA256 first + // SECURITY: Reject high-S signatures (matches aptos-core behavior) + if signature.inner.normalize_s().is_some() { + return Err(AptosError::SignatureVerificationFailed); + } let hash = crate::crypto::sha2_256(message); self.inner .verify(&hash, &signature.inner) @@ -245,14 +267,24 @@ impl Secp256k1PublicKey { /// Verifies a signature against a pre-hashed message. /// + /// # Security + /// + /// Rejects high-S signatures before verification, matching Aptos on-chain + /// behavior. + /// /// # Errors /// - /// Returns [`AptosError::SignatureVerificationFailed`] if the signature is invalid or does not match the hash. + /// Returns [`AptosError::SignatureVerificationFailed`] if the signature has + /// a high-S value, is invalid, or does not match the hash. pub fn verify_prehashed( &self, hash: &[u8; 32], signature: &Secp256k1Signature, ) -> AptosResult<()> { + // SECURITY: Reject high-S signatures (matches aptos-core behavior) + if signature.inner.normalize_s().is_some() { + return Err(AptosError::SignatureVerificationFailed); + } self.inner .verify(hash, &signature.inner) .map_err(|_| AptosError::SignatureVerificationFailed) @@ -344,11 +376,18 @@ pub struct Secp256k1Signature { impl Secp256k1Signature { /// Creates a signature from raw bytes (64 bytes, r || s). /// + /// # Security + /// + /// Rejects high-S signatures to match Aptos on-chain verification behavior. + /// The Aptos VM only accepts low-S (canonical) ECDSA signatures to prevent + /// signature malleability attacks. + /// /// # Errors /// /// Returns [`AptosError::InvalidSignature`] if: /// - The byte slice length is not exactly 64 bytes /// - The bytes do not represent a valid Secp256k1 signature + /// - The signature has a high-S value (not canonical) pub fn from_bytes(bytes: &[u8]) -> AptosResult { if bytes.len() != SECP256K1_SIGNATURE_LENGTH { return Err(AptosError::InvalidSignature(format!( @@ -359,6 +398,15 @@ impl Secp256k1Signature { } let signature = K256Signature::from_slice(bytes) .map_err(|e| AptosError::InvalidSignature(e.to_string()))?; + // SECURITY: Reject high-S signatures. Aptos on-chain verification only + // accepts low-S (canonical) signatures to prevent malleability. + // normalize_s() returns Some(_) if the signature was high-S. + if signature.normalize_s().is_some() { + return Err(AptosError::InvalidSignature( + "high-S signature rejected: Aptos requires low-S (canonical) ECDSA signatures" + .into(), + )); + } Ok(Self { inner: signature }) } @@ -547,6 +595,58 @@ mod tests { assert!(result.is_err()); } + #[test] + fn test_high_s_signature_rejected() { + use k256::elliptic_curve::ops::Neg; + + // Sign a message (produces low-S) + let private_key = Secp256k1PrivateKey::generate(); + let signature = private_key.sign(b"test message"); + + // Construct high-S by negating the S component: S' = n - S + let low_s_sig = k256::ecdsa::Signature::from_slice(&signature.to_bytes()).unwrap(); + let (r, s) = low_s_sig.split_scalars(); + let neg_s = s.neg(); + let high_s_sig = k256::ecdsa::Signature::from_scalars(r, neg_s).unwrap(); + // Confirm it really is high-S (normalize_s returns Some for high-S) + assert!( + high_s_sig.normalize_s().is_some(), + "constructed signature should be high-S" + ); + let high_s_bytes = high_s_sig.to_bytes(); + + // from_bytes should reject high-S + let result = Secp256k1Signature::from_bytes(&high_s_bytes); + assert!(result.is_err(), "high-S signature should be rejected"); + assert!( + result + .unwrap_err() + .to_string() + .contains("high-S signature rejected"), + "error message should mention high-S rejection" + ); + + // Verify should also reject high-S (defense-in-depth via inner field) + let sig_with_high_s = Secp256k1Signature { inner: high_s_sig }; + let public_key = private_key.public_key(); + let result = public_key.verify(b"test message", &sig_with_high_s); + assert!(result.is_err(), "verify should reject high-S signature"); + } + + #[test] + fn test_signing_always_produces_low_s() { + // Run multiple iterations to increase confidence + for _ in 0..20 { + let private_key = Secp256k1PrivateKey::generate(); + let signature = private_key.sign(b"test low-s"); + // normalize_s returns None if already low-S + assert!( + signature.inner.normalize_s().is_none(), + "signing must always produce low-S signatures" + ); + } + } + #[test] fn test_json_serialization_public_key() { let private_key = Secp256k1PrivateKey::generate(); diff --git a/crates/aptos-sdk/src/crypto/secp256r1.rs b/crates/aptos-sdk/src/crypto/secp256r1.rs index 077bdf8..32db6cc 100644 --- a/crates/aptos-sdk/src/crypto/secp256r1.rs +++ b/crates/aptos-sdk/src/crypto/secp256r1.rs @@ -5,14 +5,15 @@ //! //! # Security: Signature Malleability //! -//! This implementation uses the `p256` crate which produces normalized (low-S) -//! ECDSA signatures by default. This prevents signature malleability attacks -//! where an attacker could create an alternative valid signature `(r, -s mod n)` -//! for the same message. +//! This implementation enforces **low-S only** signatures to match the Aptos +//! blockchain's on-chain verification, which rejects high-S signatures to +//! prevent signature malleability attacks. //! -//! When parsing external signatures via [`Secp256r1Signature::from_bytes`], the -//! `p256` crate accepts both low-S and high-S signatures. If strict low-S -//! enforcement is required, callers should normalize or reject high-S signatures. +//! - **Signing** always produces low-S signatures (normalized by this SDK, +//! independent of the `p256` crate's default behavior). +//! - **Parsing** (`from_bytes`, `from_hex`) rejects high-S signatures. +//! - **Verification** also rejects high-S signatures as a defense-in-depth +//! measure. use crate::crypto::traits::{PublicKey, Signature, Signer, Verifier}; use crate::error::{AptosError, AptosResult}; @@ -122,17 +123,28 @@ impl Secp256r1PrivateKey { } } - /// Signs a message (pre-hashed with SHA256) and returns the signature. + /// Signs a message (pre-hashed with SHA256) and returns a low-S signature. + /// + /// The signature is normalized to low-S form to match Aptos on-chain + /// verification requirements. pub fn sign(&self, message: &[u8]) -> Secp256r1Signature { let hash = crate::crypto::sha2_256(message); let signature: P256Signature = self.inner.sign(&hash); - Secp256r1Signature { inner: signature } + // SECURITY: Normalize to low-S to match Aptos on-chain verification. + // The p256 crate does not guarantee low-S output from signing. + let normalized = signature.normalize_s().unwrap_or(signature); + Secp256r1Signature { inner: normalized } } - /// Signs a pre-hashed message directly. + /// Signs a pre-hashed message directly and returns a low-S signature. + /// + /// The signature is normalized to low-S form to match Aptos on-chain + /// verification requirements. pub fn sign_prehashed(&self, hash: &[u8; 32]) -> Secp256r1Signature { let signature: P256Signature = self.inner.sign(hash); - Secp256r1Signature { inner: signature } + // SECURITY: Normalize to low-S to match Aptos on-chain verification. + let normalized = signature.normalize_s().unwrap_or(signature); + Secp256r1Signature { inner: normalized } } } @@ -231,10 +243,21 @@ impl Secp256r1PublicKey { /// Verifies a signature against a message. /// + /// # Security + /// + /// Rejects high-S signatures before verification, matching Aptos on-chain + /// behavior. This is a defense-in-depth check; signatures created through + /// this SDK's `from_bytes` are already guaranteed to be low-S. + /// /// # Errors /// - /// Returns [`AptosError::SignatureVerificationFailed`] if the signature is invalid or does not match the message. + /// Returns [`AptosError::SignatureVerificationFailed`] if the signature has + /// a high-S value, is invalid, or does not match the message. pub fn verify(&self, message: &[u8], signature: &Secp256r1Signature) -> AptosResult<()> { + // SECURITY: Reject high-S signatures (matches aptos-core behavior) + if signature.inner.normalize_s().is_some() { + return Err(AptosError::SignatureVerificationFailed); + } let hash = crate::crypto::sha2_256(message); self.inner .verify(&hash, &signature.inner) @@ -243,14 +266,24 @@ impl Secp256r1PublicKey { /// Verifies a signature against a pre-hashed message. /// + /// # Security + /// + /// Rejects high-S signatures before verification, matching Aptos on-chain + /// behavior. + /// /// # Errors /// - /// Returns [`AptosError::SignatureVerificationFailed`] if the signature is invalid or does not match the hash. + /// Returns [`AptosError::SignatureVerificationFailed`] if the signature has + /// a high-S value, is invalid, or does not match the hash. pub fn verify_prehashed( &self, hash: &[u8; 32], signature: &Secp256r1Signature, ) -> AptosResult<()> { + // SECURITY: Reject high-S signatures (matches aptos-core behavior) + if signature.inner.normalize_s().is_some() { + return Err(AptosError::SignatureVerificationFailed); + } self.inner .verify(hash, &signature.inner) .map_err(|_| AptosError::SignatureVerificationFailed) @@ -342,11 +375,18 @@ pub struct Secp256r1Signature { impl Secp256r1Signature { /// Creates a signature from raw bytes (64 bytes, r || s). /// + /// # Security + /// + /// Rejects high-S signatures to match Aptos on-chain verification behavior. + /// The Aptos VM only accepts low-S (canonical) ECDSA signatures to prevent + /// signature malleability attacks. + /// /// # Errors /// /// Returns [`AptosError::InvalidSignature`] if: /// - The byte slice length is not exactly 64 bytes /// - The bytes do not represent a valid Secp256r1 signature + /// - The signature has a high-S value (not canonical) pub fn from_bytes(bytes: &[u8]) -> AptosResult { if bytes.len() != SECP256R1_SIGNATURE_LENGTH { return Err(AptosError::InvalidSignature(format!( @@ -357,6 +397,15 @@ impl Secp256r1Signature { } let signature = P256Signature::from_slice(bytes) .map_err(|e| AptosError::InvalidSignature(e.to_string()))?; + // SECURITY: Reject high-S signatures. Aptos on-chain verification only + // accepts low-S (canonical) signatures to prevent malleability. + // normalize_s() returns Some(_) if the signature was high-S. + if signature.normalize_s().is_some() { + return Err(AptosError::InvalidSignature( + "high-S signature rejected: Aptos requires low-S (canonical) ECDSA signatures" + .into(), + )); + } Ok(Self { inner: signature }) } @@ -533,6 +582,58 @@ mod tests { assert!(result.is_err()); } + #[test] + fn test_high_s_signature_rejected() { + use p256::elliptic_curve::ops::Neg; + + // Sign a message (produces low-S after normalization) + let private_key = Secp256r1PrivateKey::generate(); + let signature = private_key.sign(b"test message"); + + // Construct high-S by negating the S component: S' = n - S + let low_s_sig = P256Signature::from_slice(&signature.to_bytes()).unwrap(); + let (r, s) = low_s_sig.split_scalars(); + let neg_s = s.neg(); + let high_s_sig = P256Signature::from_scalars(r, neg_s).unwrap(); + // Confirm it really is high-S (normalize_s returns Some for high-S) + assert!( + high_s_sig.normalize_s().is_some(), + "constructed signature should be high-S" + ); + let high_s_bytes = high_s_sig.to_bytes(); + + // from_bytes should reject high-S + let result = Secp256r1Signature::from_bytes(&high_s_bytes); + assert!(result.is_err(), "high-S signature should be rejected"); + assert!( + result + .unwrap_err() + .to_string() + .contains("high-S signature rejected"), + "error message should mention high-S rejection" + ); + + // Verify should also reject high-S (defense-in-depth via inner field) + let sig_with_high_s = Secp256r1Signature { inner: high_s_sig }; + let public_key = private_key.public_key(); + let result = public_key.verify(b"test message", &sig_with_high_s); + assert!(result.is_err(), "verify should reject high-S signature"); + } + + #[test] + fn test_signing_always_produces_low_s() { + // Run multiple iterations to increase confidence + for _ in 0..20 { + let private_key = Secp256r1PrivateKey::generate(); + let signature = private_key.sign(b"test low-s"); + // normalize_s returns None if already low-S + assert!( + signature.inner.normalize_s().is_none(), + "signing must always produce low-S signatures" + ); + } + } + #[test] fn test_json_serialization_public_key() { let private_key = Secp256r1PrivateKey::generate(); diff --git a/crates/aptos-sdk/src/error.rs b/crates/aptos-sdk/src/error.rs index c491cdf..4410571 100644 --- a/crates/aptos-sdk/src/error.rs +++ b/crates/aptos-sdk/src/error.rs @@ -13,6 +13,21 @@ pub type AptosResult = Result; /// /// This enum covers all possible error conditions that can occur when /// interacting with the Aptos blockchain through this SDK. +/// +/// # Security: Logging Errors +/// +/// **WARNING:** The `Display` implementation on this type may include sensitive +/// information (e.g., partial key material, JWT tokens, or mnemonic phrases) in +/// its output. When logging errors, always use [`sanitized_message()`](AptosError::sanitized_message) +/// instead of `to_string()` or `Display`: +/// +/// ```rust,ignore +/// // WRONG - may leak sensitive data: +/// log::error!("Failed: {}", err); +/// +/// // CORRECT - redacts sensitive patterns: +/// log::error!("Failed: {}", err.sanitized_message()); +/// ``` #[derive(Error, Debug)] pub enum AptosError { /// Error occurred during HTTP communication @@ -157,6 +172,11 @@ pub enum AptosError { const MAX_ERROR_MESSAGE_LENGTH: usize = 1000; /// Patterns that might indicate sensitive information in error messages. +/// +/// # Security +/// +/// This list is used by [`AptosError::sanitized_message()`] to redact potentially +/// sensitive content from error messages before logging. The check is case-insensitive. const SENSITIVE_PATTERNS: &[&str] = &[ "private_key", "secret", @@ -165,6 +185,14 @@ const SENSITIVE_PATTERNS: &[&str] = &[ "seed", "bearer", "authorization", + "token", + "jwt", + "credential", + "api_key", + "apikey", + "access_token", + "refresh_token", + "pepper", ]; impl AptosError { @@ -270,11 +298,36 @@ impl AptosError { } } - // Truncate if too long + // SECURITY: Redact URLs with query parameters, which may contain API keys + // or other credentials not caught by keyword patterns above. + // e.g., reqwest errors include the request URL. + // Only redact when '?' appears within a URL token (after the scheme), + // not just anywhere in the message. + for scheme in ["http://", "https://"] { + if let Some(scheme_pos) = lower.find(scheme) { + // Look for '?' after the scheme, within the URL token + // (URLs end at whitespace or common delimiters) + let url_start = scheme_pos; + let url_rest = &lower[url_start..]; + let url_end = url_rest + .find(|c: char| c.is_whitespace() || c == '>' || c == '"' || c == '\'') + .unwrap_or(url_rest.len()); + let url_token = &url_rest[..url_end]; + if url_token.contains('?') { + return "[REDACTED: message contained URL with query parameters]".into(); + } + } + } + + // Truncate if too long (find a valid UTF-8 boundary to avoid panic) if cleaned.len() > MAX_ERROR_MESSAGE_LENGTH { + let mut end = MAX_ERROR_MESSAGE_LENGTH; + while end > 0 && !cleaned.is_char_boundary(end) { + end -= 1; + } format!( "{}... [truncated, total length: {}]", - &cleaned[..MAX_ERROR_MESSAGE_LENGTH], + &cleaned[..end], cleaned.len() ) } else { diff --git a/crates/aptos-sdk/src/retry.rs b/crates/aptos-sdk/src/retry.rs index 116bbdb..646c863 100644 --- a/crates/aptos-sdk/src/retry.rs +++ b/crates/aptos-sdk/src/retry.rs @@ -25,6 +25,7 @@ use crate::error::{AptosError, AptosResult}; use std::collections::HashSet; use std::future::Future; +use std::sync::Arc; use std::time::Duration; use tokio::time::sleep; @@ -254,12 +255,19 @@ impl RetryConfigBuilder { /// Executes an async operation with automatic retry. #[derive(Debug, Clone)] pub struct RetryExecutor { - config: RetryConfig, + config: Arc, } impl RetryExecutor { /// Creates a new retry executor with the given config. pub fn new(config: RetryConfig) -> Self { + Self { + config: Arc::new(config), + } + } + + /// Creates a retry executor from a shared config, avoiding a clone. + pub fn from_shared(config: Arc) -> Self { Self { config } } diff --git a/crates/aptos-sdk/src/transaction/batch.rs b/crates/aptos-sdk/src/transaction/batch.rs index 041ee2b..669ef71 100644 --- a/crates/aptos-sdk/src/transaction/batch.rs +++ b/crates/aptos-sdk/src/transaction/batch.rs @@ -529,29 +529,27 @@ impl BatchSummary { #[allow(missing_debug_implementations)] // Contains references that may not implement Debug pub struct BatchOperations<'a> { client: &'a FullnodeClient, - chain_id: &'a std::sync::RwLock, + chain_id: &'a std::sync::atomic::AtomicU8, } impl<'a> BatchOperations<'a> { /// Creates a new batch operations helper. - pub fn new(client: &'a FullnodeClient, chain_id: &'a std::sync::RwLock) -> Self { + pub fn new(client: &'a FullnodeClient, chain_id: &'a std::sync::atomic::AtomicU8) -> Self { Self { client, chain_id } } /// Resolves the chain ID, fetching from the node if unknown. async fn resolve_chain_id(&self) -> AptosResult { - { - let chain_id = self.chain_id.read().expect("chain_id lock poisoned"); - if chain_id.id() > 0 { - return Ok(*chain_id); - } + let id = self.chain_id.load(std::sync::atomic::Ordering::Relaxed); + if id > 0 { + return Ok(ChainId::new(id)); } // Chain ID is unknown; fetch from node let response = self.client.get_ledger_info().await?; let info = response.into_inner(); - let new_chain_id = ChainId::new(info.chain_id); - *self.chain_id.write().expect("chain_id lock poisoned") = new_chain_id; - Ok(new_chain_id) + self.chain_id + .store(info.chain_id, std::sync::atomic::Ordering::Relaxed); + Ok(ChainId::new(info.chain_id)) } /// Builds a batch of transactions for an account. diff --git a/crates/aptos-sdk/src/transaction/sponsored.rs b/crates/aptos-sdk/src/transaction/sponsored.rs index 62f36dd..6085873 100644 --- a/crates/aptos-sdk/src/transaction/sponsored.rs +++ b/crates/aptos-sdk/src/transaction/sponsored.rs @@ -213,13 +213,13 @@ impl SponsoredTransactionBuilder { .fee_payer_address .ok_or_else(|| AptosError::transaction("fee_payer is required"))?; + // SECURITY: Apply expiration offset only once (was previously doubled) let expiration_timestamp_secs = self.expiration_timestamp_secs.unwrap_or_else(|| { SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap_or_default() .as_secs() .saturating_add(DEFAULT_EXPIRATION_SECONDS) - + DEFAULT_EXPIRATION_SECONDS }); let raw_txn = RawTransaction::new(