Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
426 changes: 426 additions & 0 deletions SECURITY_AUDIT.md

Large diffs are not rendered by default.

133 changes: 128 additions & 5 deletions crates/aptos-sdk-macros/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<proc_macro2::Ident, TokenStream> {
if let Err(e) = validate_rust_ident(name) {
return Err(syn::Error::new(proc_macro2::Span::call_site(), e).to_compile_error());
}
Comment on lines +32 to +96
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

safe_format_ident can still panic on Rust keywords because validate_rust_ident only checks character classes; format_ident!("{}", "fn") (or mod, crate, etc.) will panic inside proc_macro2::Ident::new. Since this helper is explicitly meant to prevent panics on malformed ABI, add a keyword check and either return a compile error or generate a raw identifier (Ident::new_raw / r#...).

Suggested change
/// Safely creates a `format_ident!` from a string, returning a compile error if invalid.
fn safe_format_ident(name: &str) -> Result<proc_macro2::Ident, TokenStream> {
if let Err(e) = validate_rust_ident(name) {
return Err(syn::Error::new(proc_macro2::Span::call_site(), e).to_compile_error());
}
/// Returns true if `name` is a reserved Rust keyword.
fn is_rust_keyword(name: &str) -> bool {
matches!(
name,
// Strict keywords
"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"
// 2018+ keywords and common contextual keywords that are rejected by `Ident::new`
| "async"
| "await"
| "dyn"
| "abstract"
| "become"
| "box"
| "do"
| "final"
| "macro"
| "override"
| "priv"
| "try"
| "typeof"
| "unsized"
| "virtual"
| "yield"
)
}
/// Safely creates a `format_ident!` from a string, returning a compile error if invalid.
fn safe_format_ident(name: &str) -> Result<proc_macro2::Ident, TokenStream> {
if let Err(e) = validate_rust_ident(name) {
return Err(syn::Error::new(proc_macro2::Span::call_site(), e).to_compile_error());
}
// If this is a Rust keyword, construct a raw identifier to avoid panics from `Ident::new`.
if is_rust_keyword(name) {
return Ok(proc_macro2::Ident::new_raw(
name,
proc_macro2::Span::call_site(),
));
}

Copilot uses AI. Check for mistakes.
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,
Expand Down Expand Up @@ -118,15 +216,25 @@ 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);

let fields: Vec<_> = struct_def
.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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
41 changes: 40 additions & 1 deletion crates/aptos-sdk-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Comment on lines +118 to +154
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path traversal protection has a security gap: if either manifest_path.canonicalize() or file_path.canonicalize() fails (returns Err), the validation is silently skipped and the file read proceeds. An attacker could exploit this by referencing a path that doesn't exist yet but would be created later, or by triggering permission errors that prevent canonicalization.

Consider handling canonicalization failures explicitly:

let canonical_manifest = manifest_path.canonicalize().map_err(|e| {
    syn::Error::new(
        input.name.span(),
        format!("Failed to resolve project directory: {e}")
    )
})?;
let canonical_file = file_path.canonicalize().map_err(|e| {
    syn::Error::new(
        input.name.span(),
        format!("Failed to resolve ABI file path: {e}")
    )
})?;
if !canonical_file.starts_with(&canonical_manifest) {
    return syn::Error::new(...).to_compile_error().into();
}

This ensures the validation always runs and provides clear error messages when canonicalization fails.

Copilot uses AI. Check for mistakes.

let abi_content = match std::fs::read_to_string(&file_path) {
Ok(content) => content,
Expand Down
7 changes: 6 additions & 1 deletion crates/aptos-sdk/examples/account_management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
80 changes: 72 additions & 8 deletions crates/aptos-sdk/src/account/keyless.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
};
Comment on lines +138 to +161
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from_issuer docs say “Only HTTPS issuers are accepted”, but the implementation still returns OidcProvider::Custom for non-HTTPS issuers (with an empty jwks_url) and only fails later when fetching/parsing. Please either (a) adjust the docs to clarify that non-HTTPS issuers result in a provider that will error at JWKS fetch time, or (b) change the API to return a Result (if acceptable) so rejection happens at construction time.

Copilot uses AI. Check for mistakes.
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<u8>);

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<u8>) -> Self {
Expand Down Expand Up @@ -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)
})
}
Expand Down Expand Up @@ -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)
})
}
Expand Down Expand Up @@ -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
Expand All @@ -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<JwkSet> {
// 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?;

Expand All @@ -735,7 +795,11 @@ async fn fetch_jwks(client: &reqwest::Client, jwks_url: &str) -> AptosResult<Jwk
)));
}

let jwks: JwkSet = response.json().await?;
// SECURITY: Stream body with size limit to prevent OOM from a
// compromised or malicious JWKS endpoint (including chunked encoding).
let bytes = crate::config::read_response_bounded(response, MAX_JWKS_RESPONSE_SIZE).await?;
let jwks: JwkSet = serde_json::from_slice(&bytes)
.map_err(|e| AptosError::InvalidJwt(format!("failed to parse JWKS: {e}")))?;
Ok(jwks)
}

Expand Down
Loading
Loading