From 80bb9c04936b4720067e3727da698698faa45628 Mon Sep 17 00:00:00 2001 From: Sharad Boni Date: Wed, 15 Apr 2026 15:50:43 -0700 Subject: [PATCH 1/2] Fix ReDoS in RFC3966 domain validation and integer underflow in matcher Bug A: The rfc3966_domainlabel_ and rfc3966_toplabel_ regex patterns used nested quantifiers of the form `[chars]+((\\-)*[chars])*` which cause O(N^2) backtracking on non-matching inputs via ICU regex. This is exploitable through the phone-context parameter in RFC3966 URIs parsed by IsPhoneContextValid(). Rewrite patterns to `[chars]+(-[chars]+)*` which requires at least one character after each hyphen, eliminating quantifier ambiguity. Bug B: In AllNumberGroupsAreExactlyPresent(), when candidate_groups is empty, `candidate_groups.size() - 1` underflows size_t (unsigned) producing a huge value, leading to undefined behavior when cast to int and used as a vector index. Add an early return guard. --- cpp/src/phonenumbers/phonenumbermatcher.cc | 5 +++++ cpp/src/phonenumbers/phonenumberutil.cc | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/cpp/src/phonenumbers/phonenumbermatcher.cc b/cpp/src/phonenumbers/phonenumbermatcher.cc index 2d60ceb4d1..60da8fcfed 100644 --- a/cpp/src/phonenumbers/phonenumbermatcher.cc +++ b/cpp/src/phonenumbers/phonenumbermatcher.cc @@ -837,6 +837,11 @@ bool PhoneNumberMatcher::AllNumberGroupsAreExactlyPresent( candidate_groups.push_back(digit_block); } + // Guard against size_t underflow when candidate_groups is empty. + if (candidate_groups.empty()) { + return false; + } + // Set this to the last group, skipping it if the number has an extension. int candidate_number_group_index = static_cast( phone_number.has_extension() ? candidate_groups.size() - 2 diff --git a/cpp/src/phonenumbers/phonenumberutil.cc b/cpp/src/phonenumbers/phonenumberutil.cc index c0c9d09a04..71f5afe16c 100644 --- a/cpp/src/phonenumbers/phonenumberutil.cc +++ b/cpp/src/phonenumbers/phonenumberutil.cc @@ -790,9 +790,9 @@ class PhoneNumberRegExpsAndMappings { StrCat("(", kDigits, "|", kRfc3966VisualSeparator, ")")), alphanum_(StrCat(kValidAlphaInclUppercase, kDigits)), rfc3966_domainlabel_( - StrCat("[", alphanum_, "]+((\\-)*[", alphanum_, "])*")), + StrCat("[", alphanum_, "]+(-[", alphanum_, "]+)*")), rfc3966_toplabel_(StrCat("[", kValidAlphaInclUppercase, - "]+((\\-)*[", alphanum_, "])*")), + "]+(-[", alphanum_, "]+)*")), regexp_factory_(new RegExpFactory()), regexp_cache_(new RegExpCache(*regexp_factory_.get(), 128)), diallable_char_mappings_(), From 797924bf8ef3793535686a2c4c32a1d6ffe54cc1 Mon Sep 17 00:00:00 2001 From: Sharad Boni Date: Wed, 15 Apr 2026 23:08:56 -0700 Subject: [PATCH 2/2] Fix regex to allow consecutive hyphens in domain labels The previous fix changed the domain label regex from [chars]+((\\-)*[chars])* to [chars]+(-[chars]+)*, which broke parsing of phone-context values like 'a--z' that contain consecutive hyphens (valid per RFC). Use -+ instead of - to allow one or more hyphens between alphanumeric groups, while still preventing ReDoS since hyphen and alphanum character classes are disjoint. --- cpp/src/phonenumbers/phonenumberutil.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/phonenumbers/phonenumberutil.cc b/cpp/src/phonenumbers/phonenumberutil.cc index 71f5afe16c..bf245e5b31 100644 --- a/cpp/src/phonenumbers/phonenumberutil.cc +++ b/cpp/src/phonenumbers/phonenumberutil.cc @@ -790,9 +790,9 @@ class PhoneNumberRegExpsAndMappings { StrCat("(", kDigits, "|", kRfc3966VisualSeparator, ")")), alphanum_(StrCat(kValidAlphaInclUppercase, kDigits)), rfc3966_domainlabel_( - StrCat("[", alphanum_, "]+(-[", alphanum_, "]+)*")), + StrCat("[", alphanum_, "]+(-+[", alphanum_, "]+)*")), rfc3966_toplabel_(StrCat("[", kValidAlphaInclUppercase, - "]+(-[", alphanum_, "]+)*")), + "]+(-+[", alphanum_, "]+)*")), regexp_factory_(new RegExpFactory()), regexp_cache_(new RegExpCache(*regexp_factory_.get(), 128)), diallable_char_mappings_(),