From 03e775947d044da3938b8a3a8a31e3359f955538 Mon Sep 17 00:00:00 2001 From: Chris Date: Fri, 9 Oct 2026 06:18:27 -0700 Subject: [PATCH] Merge commit from fork --- crates/policy/src/policy/variables.rs | 108 +++++++++++++++++++++----- 1 file changed, 89 insertions(+), 19 deletions(-) diff --git a/crates/policy/src/policy/variables.rs b/crates/policy/src/policy/variables.rs index ce7d30a53..f5f438e93 100644 --- a/crates/policy/src/policy/variables.rs +++ b/crates/policy/src/policy/variables.rs @@ -252,6 +252,13 @@ impl PolicyVariableResolver for VariableResolver { } pub async fn resolve_aws_variables(pattern: &str, resolver: &dyn PolicyVariableResolver) -> Vec { + resolve_aws_variables_with_depth(pattern, resolver, 0).await +} + +async fn resolve_aws_variables_with_depth(pattern: &str, resolver: &dyn PolicyVariableResolver, depth: usize) -> Vec { + if depth >= 10 { + return vec![pattern.to_string()]; + } let mut results = vec![pattern.to_string()]; let mut changed = true; @@ -264,7 +271,7 @@ pub async fn resolve_aws_variables(pattern: &str, resolver: &dyn PolicyVariableR let mut new_results = Vec::new(); for result in &results { - let resolved = resolve_single_pass(result, resolver).await; + let resolved = resolve_single_pass(result, resolver, depth).await; if resolved.len() > 1 || (resolved.len() == 1 && &resolved[0] != result) { changed = true; } @@ -288,30 +295,31 @@ pub async fn resolve_aws_variables(pattern: &str, resolver: &dyn PolicyVariableR fn resolve_aws_variables_boxed<'a>( pattern: &'a str, resolver: &'a dyn PolicyVariableResolver, + depth: usize, ) -> std::pin::Pin> + Send + 'a>> { - Box::pin(resolve_aws_variables(pattern, resolver)) + Box::pin(resolve_aws_variables_with_depth(pattern, resolver, depth)) } /// Single pass resolution of variables in a string -async fn resolve_single_pass(pattern: &str, resolver: &dyn PolicyVariableResolver) -> Vec { +async fn resolve_single_pass(pattern: &str, resolver: &dyn PolicyVariableResolver, depth: usize) -> Vec { // Find all ${...} format variables - let mut results = vec![pattern.to_string()]; + let mut results = vec![(pattern.to_string(), 0)]; // Process each result string let mut i = 0; while i < results.len() { - let mut start = 0; + let mut start = results[i].1; let mut modified = false; // Find variables in current string - while let Some(pos) = results[i][start..].find("${") { + while let Some(pos) = results[i].0[start..].find("${") { let actual_pos = start + pos; // Find the matching closing brace, taking into account nested braces let mut brace_count = 1; let mut end_pos = actual_pos + 2; // Start after "${" - let bytes = results[i].as_bytes(); + let bytes = results[i].0.as_bytes(); while end_pos < bytes.len() && brace_count > 0 { match bytes[end_pos] { b'{' => brace_count += 1, @@ -324,19 +332,23 @@ async fn resolve_single_pass(pattern: &str, resolver: &dyn PolicyVariableResolve } if brace_count == 0 { - let var_name = &results[i][actual_pos + 2..end_pos]; + let var_name = &results[i].0[actual_pos + 2..end_pos]; // Check if this is a nested variable (contains ${...} inside) if var_name.contains("${") { // For nested variables like ${${a}-${b}}, we need to resolve the inner variables first // Then use the resolved result as a new variable to resolve - let resolved_inner = resolve_aws_variables_boxed(var_name, resolver).await; + let resolved_inner = resolve_aws_variables_boxed(var_name, resolver, depth + 1).await; + if resolved_inner.len() == 1 && resolved_inner[0] == var_name { + start = end_pos + 1; + continue; + } let mut new_results = Vec::new(); for resolved_var_name in resolved_inner { - let prefix = &results[i][..actual_pos]; - let suffix = &results[i][end_pos + 1..]; - new_results.push(format!("{prefix}{resolved_var_name}{suffix}")); + let prefix = &results[i].0[..actual_pos]; + let suffix = &results[i].0[end_pos + 1..]; + new_results.push((format!("{prefix}{resolved_var_name}{suffix}"), actual_pos + resolved_var_name.len())); } if !new_results.is_empty() { @@ -354,11 +366,13 @@ async fn resolve_single_pass(pattern: &str, resolver: &dyn PolicyVariableResolve if !values.is_empty() { // If there are multiple values, create a new result for each value let mut new_results = Vec::new(); - let prefix = &results[i][..actual_pos]; - let suffix = &results[i][end_pos + 1..]; + let prefix = &results[i].0[..actual_pos]; + let suffix = &results[i].0[end_pos + 1..]; for value in values { - new_results.push(format!("{prefix}{value}{suffix}")); + // Resume after the substitution; a generated placeholder + // belongs to the next bounded pass, not this one. + new_results.push((format!("{prefix}{value}{suffix}"), actual_pos + value.len())); } results.splice(i..i + 1, new_results); @@ -367,9 +381,9 @@ async fn resolve_single_pass(pattern: &str, resolver: &dyn PolicyVariableResolve } else { // Variable resolved to empty, just remove the variable placeholder let mut new_results = Vec::new(); - let prefix = &results[i][..actual_pos]; - let suffix = &results[i][end_pos + 1..]; - new_results.push(format!("{prefix}{suffix}")); + let prefix = &results[i].0[..actual_pos]; + let suffix = &results[i].0[end_pos + 1..]; + new_results.push((format!("{prefix}{suffix}"), actual_pos)); results.splice(i..i + 1, new_results); modified = true; @@ -391,7 +405,7 @@ async fn resolve_single_pass(pattern: &str, resolver: &dyn PolicyVariableResolve } } - results + results.into_iter().map(|(value, _)| value).collect() } #[cfg(test)] @@ -399,6 +413,62 @@ mod tests { use super::*; use serde_json::Value; use std::collections::HashMap; + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct CycleResolver { + calls: AtomicUsize, + } + + #[async_trait] + impl PolicyVariableResolver for CycleResolver { + async fn resolve(&self, name: &str) -> Option { + // Make the vulnerable implementation terminate so a regression + // reports an assertion failure instead of hanging the test runner. + if self.calls.fetch_add(1, Ordering::Relaxed) >= 100 { + return None; + } + match name { + "aws:AccountId" => Some("$$".to_string()), + "aws:username" => Some("{aws:AccountId}{aws:username}".to_string()), + _ => None, + } + } + + fn is_dynamic(&self, _: &str) -> bool { + false + } + } + + #[tokio::test] + async fn generated_policy_variable_cycles_are_bounded() { + let resolver = CycleResolver { + calls: AtomicUsize::new(0), + }; + let result = resolve_aws_variables("${aws:AccountId}{aws:username}", &resolver).await; + assert!(resolver.calls.load(Ordering::Relaxed) <= 10); + assert_eq!(result, vec!["${aws:AccountId}{aws:username}".to_string()]); + } + + #[tokio::test] + async fn a_single_pass_resolves_all_original_variables() { + let mut context = VariableContext::new(); + context.username = Some("alice".to_string()); + let resolver = VariableResolver::new(context); + let pattern = "${aws:username}".repeat(20); + assert_eq!(resolve_single_pass(&pattern, &resolver, 0).await, vec!["alice".repeat(20)]); + } + + #[tokio::test] + async fn nested_policy_variables_have_a_depth_limit() { + let resolver = CycleResolver { + calls: AtomicUsize::new(0), + }; + let pattern = format!("{}aws:username{}", "${".repeat(100), "}".repeat(100)); + let result = resolve_aws_variables(&pattern, &resolver).await; + assert_eq!(result.len(), 1); + assert!(result[0].contains("${")); + assert!(resolver.calls.load(Ordering::Relaxed) <= 10); + } #[tokio::test] async fn test_resolve_aws_variables_with_username() {