From bc2e713b0650c9ffd21796e47840f5e4ede1cafa Mon Sep 17 00:00:00 2001 From: Satyaki Ghosh Date: Tue, 20 Oct 2026 15:27:47 -0400 Subject: [PATCH 1/2] Fix regex pattern bug --- .../templates/bad/schema_composition.yaml | 2 +- src/schema-validator/tests/integration.rs | 8 +- src/template-model/src/pattern.rs | 167 +++++++++++++++++- 3 files changed, 165 insertions(+), 12 deletions(-) diff --git a/src/resources/templates/bad/schema_composition.yaml b/src/resources/templates/bad/schema_composition.yaml index d5428f17..71146736 100644 --- a/src/resources/templates/bad/schema_composition.yaml +++ b/src/resources/templates/bad/schema_composition.yaml @@ -1,6 +1,6 @@ AWSTemplateFormatVersion: "2010-09-09" Resources: - # F3018: oneOf - ImageBuilder needs exactly one of ImageName or ImageArn + # AppStream ImageBuilder needs at least one of ImageName or ImageArn NoImage: Type: AWS::AppStream::ImageBuilder Properties: diff --git a/src/schema-validator/tests/integration.rs b/src/schema-validator/tests/integration.rs index 2197b497..ef6d689a 100644 --- a/src/schema-validator/tests/integration.rs +++ b/src/schema-validator/tests/integration.rs @@ -623,11 +623,11 @@ fn type_mismatch_array_for_object() { } #[test] -fn composition_f3018_one_of_zero_matches() { +fn composition_f3017_any_of_zero_matches() { let diags = validate_fixture("bad/schema_composition.yaml"); - let f3018: Vec<_> = - diags.iter().filter(|d| d.rule_id == "F3018" && d.resource_logical_id() == Some("NoImage")).collect(); - assert!(!f3018.is_empty(), "expected F3018 for ImageBuilder missing both ImageName and ImageArn"); + let f3017: Vec<_> = + diags.iter().filter(|d| d.rule_id == "F3017" && d.resource_logical_id() == Some("NoImage")).collect(); + assert!(!f3017.is_empty(), "expected F3017 for AppStream ImageBuilder missing both ImageName and ImageArn"); } #[test] diff --git a/src/template-model/src/pattern.rs b/src/template-model/src/pattern.rs index cd77adb5..3b16740c 100644 --- a/src/template-model/src/pattern.rs +++ b/src/template-model/src/pattern.rs @@ -8,11 +8,12 @@ //! //! [`compile`] closes that gap. It tries, cheapest first: the `regex` crate with a raised //! compiled-size limit, then a normalization pass that rewrites Rust-incompatible-but-equivalent -//! syntax, then the `fancy-regex` backtracking engine (which understands lookaround/backreferences) -//! on the raw and normalized forms. Every pattern shipped in the compiled schemas - and every -//! service-valid `AllowedPattern` - compiles through one of these paths, so a constraint is never -//! silently discarded. [`is_service_valid`] reports whether a pattern is enforceable this way, for -//! the rule that flags genuinely-malformed `AllowedPattern`s. +//! syntax, then the `fancy-regex` backtracking engine on the raw and normalized forms. A final +//! fallback rewrites oversized bounded-dot repetitions into an equivalent compact form for the +//! backtracking engine. Every pattern shipped in the compiled schemas - and every service-valid +//! `AllowedPattern` - compiles through one of these paths, so a constraint is never silently +//! discarded. [`is_service_valid`] reports whether a pattern is enforceable this way, for the rule +//! that flags genuinely-malformed `AllowedPattern`s. use std::collections::HashMap; use std::sync::{Arc, LazyLock, RwLock}; @@ -80,13 +81,20 @@ fn compile_uncached(pattern: &str) -> Option { return Some(CompiledPattern::Fast(re)); } let normalized = normalize(pattern); - if let Ok(re) = build_fast(&normalized) { + let normalization_changed_pattern = normalized != pattern; + if normalization_changed_pattern && let Ok(re) = build_fast(&normalized) { return Some(CompiledPattern::Fast(re)); } if let Ok(re) = fancy_regex::Regex::new(pattern) { return Some(CompiledPattern::Fancy(re)); } - if let Ok(re) = fancy_regex::Regex::new(&normalized) { + if normalization_changed_pattern && let Ok(re) = fancy_regex::Regex::new(&normalized) { + return Some(CompiledPattern::Fancy(re)); + } + let compact_bounded_dots = rewrite_bounded_dot_repetitions(&normalized); + if compact_bounded_dots != normalized + && let Ok(re) = fancy_regex::Regex::new(&compact_bounded_dots) + { return Some(CompiledPattern::Fancy(re)); } None @@ -148,6 +156,89 @@ fn normalize(pattern: &str) -> String { QUANTIFIED_LOOKAROUND.replace_all(&out, "$1").into_owned() } +const COMPACT_DOT_ATOM: &str = r"(?:(?=.)(?s:.))"; + +/// Large bounded repetitions of `.` exceed the fast engine's automaton size limit. The lookahead +/// evaluates the ordinary dot under its surrounding flags, while the dot-all atom consumes exactly +/// one scalar and makes the backtracking engine retain the repetition as a compact counter. Only a +/// single repetition is rewritten so this fallback cannot newly admit adjacent broad repetitions +/// whose ambiguous partitions may exhaust the backtracking budget. +fn rewrite_bounded_dot_repetitions(pattern: &str) -> String { + let chars: Vec = pattern.chars().collect(); + let mut out = String::with_capacity(pattern.len()); + let mut index = 0; + let mut inside_class = false; + let mut has_rewritten_dot = false; + while index < chars.len() { + let character = chars[index]; + if character == '\\' && index + 1 < chars.len() { + out.push(character); + out.push(chars[index + 1]); + index += 2; + continue; + } + if character == '[' && !inside_class { + inside_class = true; + out.push(character); + index += 1; + if chars.get(index) == Some(&'^') { + out.push('^'); + index += 1; + } + if chars.get(index) == Some(&']') { + out.push(']'); + index += 1; + } + continue; + } + if character == ']' && inside_class { + inside_class = false; + } else if character == '.' && !inside_class && bounded_quantifier_follows(&chars, index + 1) { + if has_rewritten_dot { + return pattern.to_string(); + } + has_rewritten_dot = true; + out.push_str(COMPACT_DOT_ATOM); + index += 1; + continue; + } + out.push(character); + index += 1; + } + out +} + +fn bounded_quantifier_follows(chars: &[char], index: usize) -> bool { + if chars.get(index) != Some(&'{') { + return false; + } + + let mut cursor = index + 1; + let Some(minimum) = parse_quantifier_bound(chars, &mut cursor) else { + return false; + }; + + match chars.get(cursor) { + Some('}') => true, + Some(',') => { + cursor += 1; + parse_quantifier_bound(chars, &mut cursor) + .is_some_and(|maximum| minimum <= maximum && chars.get(cursor) == Some(&'}')) + } + _ => false, + } +} + +fn parse_quantifier_bound(chars: &[char], cursor: &mut usize) -> Option { + let start = *cursor; + let mut bound = 0_u64; + while let Some(character) = chars.get(*cursor).filter(|character| character.is_ascii_digit()) { + bound = bound.checked_mul(10)?.checked_add(u64::from(character.to_digit(10)?))?; + *cursor += 1; + } + (*cursor > start).then_some(bound) +} + /// `\Z` in Python matches at end-of-string or just before a trailing newline; `\z` in Rust matches /// only at the absolute end. CloudFormation validates single-line scalar values that never carry a /// trailing newline, so the two are equivalent for every real input. @@ -458,6 +549,67 @@ mod tests { assert!(compiled.is_match("valid.name-1")); } + #[test] + fn bounded_dot_pattern_uses_other_normalizations_before_the_compact_fallback() { + let compiled = compile(r"\A.{1,64}\Z").expect("normalized bounded dot pattern compiles"); + assert!(matches!(*compiled, CompiledPattern::Fast(_))); + assert!(!compiled.is_match("line\n")); + assert_eq!(normalize(r"^(?=.{1,5})+abc$"), r"^(?=.{1,5})abc$"); + } + + #[test] + fn large_bounded_dot_repetition_preserves_length_anchors_and_dot_semantics() { + const MAXIMUM_LENGTH: usize = 65_535; + + let compiled = compile(r"^.{1,65535}$").expect("large bounded dot repetition compiles"); + let CompiledPattern::Fancy(regex) = compiled.as_ref() else { + panic!("large bounded dot repetition must use the compact backtracking representation"); + }; + assert!(matches!(regex.is_match("é"), Ok(true)), "one Unicode scalar must count as one character"); + assert!(matches!(regex.is_match(""), Ok(false)), "the lower length boundary must be enforced"); + assert!( + matches!(regex.is_match("line\nbreak"), Ok(false)), + "the compact atom must preserve ordinary dot behavior" + ); + assert!( + matches!(regex.is_match("line\n"), Ok(false)), + "the compact representation must preserve the engine's anchor behavior" + ); + + let maximum_length = "a".repeat(MAXIMUM_LENGTH); + let above_maximum_length = "a".repeat(MAXIMUM_LENGTH + 1); + assert!(matches!(regex.is_match(&maximum_length), Ok(true)), "the upper length boundary must be accepted"); + assert!( + matches!(regex.is_match(&above_maximum_length), Ok(false)), + "values above the upper boundary must be rejected without exhausting the backtracking budget" + ); + + let dot_all = compile(r"(?s:^.{1,65535}$)").expect("dot-all bounded repetition compiles"); + let dot_all_matches_line_feed = match dot_all.as_ref() { + CompiledPattern::Fast(regex) => regex.is_match("\n"), + CompiledPattern::Fancy(regex) => matches!(regex.is_match("\n"), Ok(true)), + }; + assert!(dot_all_matches_line_feed, "the compiled pattern must preserve surrounding dot-all behavior"); + } + + #[test] + fn bounded_dot_rewrite_ignores_escaped_dots_and_character_classes() { + assert_eq!( + rewrite_bounded_dot_repetitions(r"^.{1,65535}\.{1,65535}[.]{1,65535}$"), + r"^(?:(?=.)(?s:.)){1,65535}\.{1,65535}[.]{1,65535}$" + ); + for character_class_pattern in [r"^[]a.{1,2}]$", r"^[^]a.{1,2}]$"] { + assert_eq!(rewrite_bounded_dot_repetitions(character_class_pattern), character_class_pattern); + } + + let adjacent_broad_repetitions = r"^.{1,30000}x.{1,30000}$"; + assert_eq!( + rewrite_bounded_dot_repetitions(adjacent_broad_repetitions), + adjacent_broad_repetitions, + "the fallback must not admit ambiguous adjacent broad repetitions" + ); + } + #[test] fn bounded_unicode_other_complement_preserves_category_and_length_constraints() { let compiled = compile(r"^[^\p{C}]{1,2048}$").expect("bounded Unicode category pattern compiles"); @@ -534,6 +686,7 @@ mod tests { fn genuinely_malformed_pattern_is_not_service_valid() { assert!(!is_service_valid(r"^(unbalanced")); assert!(!is_service_valid(r"a{2,1}")); + assert!(!is_service_valid(r"^.{2,1}$")); } #[test] From d6c3983fa1d0891475fd9850cc764ee28e68b97b Mon Sep 17 00:00:00 2001 From: Satyaki Ghosh Date: Tue, 20 Oct 2026 16:07:45 -0400 Subject: [PATCH 2/2] Fix audit --- src/Cargo.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Cargo.lock b/src/Cargo.lock index e834963b..6ed86838 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -1733,9 +1733,9 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.103.15" +version = "0.103.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3c3cf1d8b1e7d4927e2d154c3fcb02979afb9939629c62cd9048d4f07b60ac2" +checksum = "4443b1b6d39d7c663139942cc4558262562d2c84123bb6282ef177009ca8b281" dependencies = [ "ring", "rustls-pki-types", @@ -2543,9 +2543,9 @@ dependencies = [ [[package]] name = "webpki-roots" -version = "1.0.9" +version = "1.0.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7dcd9d09a39985f5344844e66b0c530a33843579125f23e21e9f0f220850f22a" +checksum = "1c1c8e20c8a38fc7d65abfe74eb7280e4aa667e8eec8bc84996d2874810748d9" dependencies = [ "rustls-pki-types", ]