From 84e77731f38b4d7d0509311d9e05de6031014283 Mon Sep 17 00:00:00 2001 From: sahvx655-wq Date: Mon, 8 Jun 2026 20:59:27 +0530 Subject: [PATCH] fix out-of-range double-to-int UB in proto map key lookup --- common/values/parsed_map_field_value.cc | 48 +++++++------ common/values/parsed_map_field_value_test.cc | 72 ++++++++++++++++++++ 2 files changed, 100 insertions(+), 20 deletions(-) diff --git a/common/values/parsed_map_field_value.cc b/common/values/parsed_map_field_value.cc index 47b737f82..641b2f0e4 100644 --- a/common/values/parsed_map_field_value.cc +++ b/common/values/parsed_map_field_value.cc @@ -33,6 +33,7 @@ #include "extensions/protobuf/internal/map_reflection.h" #include "internal/json.h" #include "internal/message_equality.h" +#include "internal/number.h" #include "internal/status_macros.h" #include "internal/well_known_types.h" #include "google/protobuf/arena.h" @@ -197,11 +198,15 @@ absl::optional ValueAsInt32(const Value& value) { uint_value && uint_value->NativeValue() <= std::numeric_limits::max()) { return static_cast(uint_value->NativeValue()); - } else if (auto double_value = value.AsDouble(); - double_value && - static_cast(static_cast( - double_value->NativeValue())) == double_value->NativeValue()) { - return static_cast(double_value->NativeValue()); + } else if (auto double_value = value.AsDouble(); double_value) { + auto number = internal::Number::FromDouble(double_value->NativeValue()); + if (number.LosslessConvertibleToInt()) { + int64_t wide_value = number.AsInt(); + if (wide_value >= std::numeric_limits::min() && + wide_value <= std::numeric_limits::max()) { + return static_cast(wide_value); + } + } } return absl::nullopt; } @@ -213,11 +218,11 @@ absl::optional ValueAsInt64(const Value& value) { uint_value && uint_value->NativeValue() <= std::numeric_limits::max()) { return static_cast(uint_value->NativeValue()); - } else if (auto double_value = value.AsDouble(); - double_value && - static_cast(static_cast( - double_value->NativeValue())) == double_value->NativeValue()) { - return static_cast(double_value->NativeValue()); + } else if (auto double_value = value.AsDouble(); double_value) { + auto number = internal::Number::FromDouble(double_value->NativeValue()); + if (number.LosslessConvertibleToInt()) { + return number.AsInt(); + } } return absl::nullopt; } @@ -231,11 +236,14 @@ absl::optional ValueAsUInt32(const Value& value) { uint_value && uint_value->NativeValue() <= std::numeric_limits::max()) { return static_cast(uint_value->NativeValue()); - } else if (auto double_value = value.AsDouble(); - double_value && - static_cast(static_cast( - double_value->NativeValue())) == double_value->NativeValue()) { - return static_cast(double_value->NativeValue()); + } else if (auto double_value = value.AsDouble(); double_value) { + auto number = internal::Number::FromDouble(double_value->NativeValue()); + if (number.LosslessConvertibleToUint()) { + uint64_t wide_value = number.AsUint(); + if (wide_value <= std::numeric_limits::max()) { + return static_cast(wide_value); + } + } } return absl::nullopt; } @@ -246,11 +254,11 @@ absl::optional ValueAsUInt64(const Value& value) { return static_cast(int_value->NativeValue()); } else if (auto uint_value = value.AsUint(); uint_value) { return uint_value->NativeValue(); - } else if (auto double_value = value.AsDouble(); - double_value && - static_cast(static_cast( - double_value->NativeValue())) == double_value->NativeValue()) { - return static_cast(double_value->NativeValue()); + } else if (auto double_value = value.AsDouble(); double_value) { + auto number = internal::Number::FromDouble(double_value->NativeValue()); + if (number.LosslessConvertibleToUint()) { + return number.AsUint(); + } } return absl::nullopt; } diff --git a/common/values/parsed_map_field_value_test.cc b/common/values/parsed_map_field_value_test.cc index 271813f40..491aab2e1 100644 --- a/common/values/parsed_map_field_value_test.cc +++ b/common/values/parsed_map_field_value_test.cc @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include @@ -225,6 +226,77 @@ TEST_F(ParsedMapFieldValueTest, Find) { IsOkAndHolds(Eq(absl::nullopt))); } +TEST_F(ParsedMapFieldValueTest, FindDoubleKeyConversion) { + // Lookups on integer-keyed maps coerce double keys following CEL's numeric + // semantics ({1: 'x'}[1.0] resolves to key 1). A double that is out of range, + // negative for an unsigned key, or non-finite must be reported as absent + // rather than narrowed to the key type: converting such a double straight to + // an integer with static_cast is undefined behaviour. + ParsedMapFieldValue int64_map( + DynamicParseTextProto(R"pb( + map_int64_int64 { key: 1 value: 100 } + )pb"), + DynamicGetField("map_int64_int64"), arena()); + EXPECT_THAT(int64_map.Find(DoubleValue(1.0), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(Optional(IntValueIs(100)))); + EXPECT_THAT(int64_map.Find(DoubleValue(1e19), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(Eq(absl::nullopt))); + EXPECT_THAT(int64_map.Find(DoubleValue(-1e19), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(Eq(absl::nullopt))); + EXPECT_THAT(int64_map.Find(DoubleValue(std::numeric_limits::infinity()), + descriptor_pool(), message_factory(), arena()), + IsOkAndHolds(Eq(absl::nullopt))); + EXPECT_THAT( + int64_map.Find(DoubleValue(std::numeric_limits::quiet_NaN()), + descriptor_pool(), message_factory(), arena()), + IsOkAndHolds(Eq(absl::nullopt))); + + ParsedMapFieldValue uint64_map( + DynamicParseTextProto(R"pb( + map_uint64_uint64 { key: 1 value: 100 } + )pb"), + DynamicGetField("map_uint64_uint64"), arena()); + EXPECT_THAT(uint64_map.Find(DoubleValue(1.0), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(Optional(UintValueIs(100)))); + EXPECT_THAT(uint64_map.Find(DoubleValue(-1.0), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(Eq(absl::nullopt))); + EXPECT_THAT(uint64_map.Find(DoubleValue(1e20), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(Eq(absl::nullopt))); + + ParsedMapFieldValue int32_map( + DynamicParseTextProto(R"pb( + map_int32_int32 { key: 1 value: 100 } + )pb"), + DynamicGetField("map_int32_int32"), arena()); + EXPECT_THAT(int32_map.Find(DoubleValue(1.0), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(Optional(IntValueIs(100)))); + EXPECT_THAT(int32_map.Find(DoubleValue(1e19), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(Eq(absl::nullopt))); + + ParsedMapFieldValue uint32_map( + DynamicParseTextProto(R"pb( + map_uint32_uint32 { key: 1 value: 100 } + )pb"), + DynamicGetField("map_uint32_uint32"), arena()); + EXPECT_THAT(uint32_map.Find(DoubleValue(1.0), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(Optional(UintValueIs(100)))); + EXPECT_THAT(uint32_map.Find(DoubleValue(-1.0), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(Eq(absl::nullopt))); + EXPECT_THAT(uint32_map.Find(DoubleValue(1e19), descriptor_pool(), + message_factory(), arena()), + IsOkAndHolds(Eq(absl::nullopt))); +} + TEST_F(ParsedMapFieldValueTest, Has) { ParsedMapFieldValue value( DynamicParseTextProto(R"pb(