From 732701823829de40bcc98ba8a7e964d4e07dd232 Mon Sep 17 00:00:00 2001 From: sayed nabhan Date: Wed, 26 Aug 2026 12:19:21 +0530 Subject: [PATCH 1/2] guard non-finite cast in log_uniform_int_distribution param_type --- absl/random/log_uniform_int_distribution.h | 13 ++++++++- .../log_uniform_int_distribution_test.cc | 27 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/absl/random/log_uniform_int_distribution.h b/absl/random/log_uniform_int_distribution.h index cbd5e0ca2ba..aac249274bf 100644 --- a/absl/random/log_uniform_int_distribution.h +++ b/absl/random/log_uniform_int_distribution.h @@ -80,7 +80,18 @@ class log_uniform_int_distribution { // which can eliminate some values depending on where the bounds fall. const double inv_log_base = 1.0 / std::log(static_cast(base_)); const double log_range = std::log(static_cast(range()) + 0.5); - log_range_ = static_cast(std::ceil(inv_log_base * log_range)); + const double result = std::ceil(inv_log_base * log_range); + // A base_ of 0 or 1, or a negative base_, violates the base_ > 1 + // precondition and leaves inv_log_base non-finite, so `result` can be + // inf or NaN. Casting such a value to int is undefined behavior; guard + // it so an out-of-contract base yields a defined (if meaningless) + // log_range_ instead. For a valid base_ (> 1), result is a small + // non-negative integer and this guard is a no-op. + log_range_ = + (result >= 0 && + result < static_cast((std::numeric_limits::max)())) + ? static_cast(result) + : 0; } } diff --git a/absl/random/log_uniform_int_distribution_test.cc b/absl/random/log_uniform_int_distribution_test.cc index 591b5b37e48..9190f0f19b4 100644 --- a/absl/random/log_uniform_int_distribution_test.cc +++ b/absl/random/log_uniform_int_distribution_test.cc @@ -116,6 +116,33 @@ TYPED_TEST(LogUniformIntDistributionTypeTest, SerializeTest) { } } +// A base of 1 (or, for signed types, a negative base) violates the base > 1 +// precondition. In debug builds the constructor asserts; in opt builds it must +// still yield a defined object rather than invoking undefined behavior while +// computing log_range_, which casts 1/log(base) * log(range) to int -- a cast +// of a non-finite double (inf/NaN) to int is UB. The bad base can also reach +// the object through operator>> reading an untrusted stream. +TYPED_TEST(LogUniformIntDistributionTypeTest, InvalidBaseIsDefinedInOptMode) { +#if defined(NDEBUG) + absl::InsecureBitGen gen; + + // Direct construction with an out-of-contract base must not invoke UB. + absl::log_uniform_int_distribution dist(0, 100, 1); + auto sample = dist(gen); + EXPECT_GE(sample, dist.min()); + EXPECT_LE(sample, dist.max()); + + // The same bad base arriving through deserialization must also stay defined. + absl::log_uniform_int_distribution after(3, 6, 17); + std::istringstream is("0 100 1"); + is >> after; + EXPECT_EQ(after.base(), static_cast(1)); + sample = after(gen); + EXPECT_GE(sample, after.min()); + EXPECT_LE(sample, after.max()); +#endif // NDEBUG +} + using log_uniform_i32 = absl::log_uniform_int_distribution; class LogUniformIntChiSquaredTest From b15918e1696ac26b2f283a26c03671a29f9b61a6 Mon Sep 17 00:00:00 2001 From: sayed nabhan Date: Sun, 13 Sep 2026 17:50:12 +0530 Subject: [PATCH 2/2] validate log_uniform_int_distribution stream input and harden preconditions Address review feedback: check the param_type preconditions with ABSL_HARDENING_ASSERT, only take the log() branch for base_ > 2 so an out-of-contract base never reaches floating-point undefined behavior (including the divide by log(1) == 0), and have operator>> validate max >= min and base > 1, setting failbit instead of constructing an invalid param_type. Replace the opt-mode out-of-contract test with one that checks operator>> rejects such input. --- absl/random/log_uniform_int_distribution.h | 37 ++++++++--------- .../log_uniform_int_distribution_test.cc | 41 ++++++++----------- 2 files changed, 35 insertions(+), 43 deletions(-) diff --git a/absl/random/log_uniform_int_distribution.h b/absl/random/log_uniform_int_distribution.h index aac249274bf..4871f0e8f60 100644 --- a/absl/random/log_uniform_int_distribution.h +++ b/absl/random/log_uniform_int_distribution.h @@ -16,13 +16,13 @@ #define ABSL_RANDOM_LOG_UNIFORM_INT_DISTRIBUTION_H_ #include -#include #include #include #include #include #include "absl/base/config.h" +#include "absl/base/macros.h" #include "absl/random/internal/iostream_state_saver.h" #include "absl/random/internal/traits.h" #include "absl/random/uniform_int_distribution.h" @@ -60,15 +60,18 @@ class log_uniform_int_distribution { range_(static_cast(max_) - static_cast(min_)), log_range_(0) { - assert(max_ >= min_); - assert(base_ > 1); + ABSL_HARDENING_ASSERT(max_ >= min_); + ABSL_HARDENING_ASSERT(base_ > 1); if (base_ == 2) { // Determine where the first set bit is on range(), giving a log2(range) // value which can be used to construct bounds. log_range_ = (std::min)(random_internal::BitWidth(range()), std::numeric_limits::digits); - } else { + } else if (base_ > 2) { + // An out-of-contract base_ (<= 1) skips this branch entirely so that + // no floating-point undefined behavior is reached. + // // NOTE: Computing the logN(x) introduces error from 2 sources: // 1. Conversion of int to double loses precision for values >= // 2^53, which may cause some log() computations to operate on @@ -80,18 +83,7 @@ class log_uniform_int_distribution { // which can eliminate some values depending on where the bounds fall. const double inv_log_base = 1.0 / std::log(static_cast(base_)); const double log_range = std::log(static_cast(range()) + 0.5); - const double result = std::ceil(inv_log_base * log_range); - // A base_ of 0 or 1, or a negative base_, violates the base_ > 1 - // precondition and leaves inv_log_base non-finite, so `result` can be - // inf or NaN. Casting such a value to int is undefined behavior; guard - // it so an out-of-contract base yields a defined (if meaningless) - // log_range_ instead. For a valid base_ (> 1), result is a small - // non-negative integer and this guard is a no-op. - log_range_ = - (result >= 0 && - result < static_cast((std::numeric_limits::max)())) - ? static_cast(result) - : 0; + log_range_ = static_cast(std::ceil(inv_log_base * log_range)); } } @@ -251,9 +243,16 @@ std::basic_istream& operator>>( auto saver = random_internal::make_istream_state_saver(is); is >> min >> max >> base; if (!is.fail()) { - x.param(param_type(static_cast(min), - static_cast(max), - static_cast(base))); + const result_type min_val = static_cast(min); + const result_type max_val = static_cast(max); + const result_type base_val = static_cast(base); + if (max_val < min_val || base_val <= 1) { + // The input violates the param_type preconditions; signal failure by + // setting the failbit instead of constructing an invalid param_type. + is.setstate(is.rdstate() | std::ios_base::failbit); + } else { + x.param(param_type(min_val, max_val, base_val)); + } } return is; } diff --git a/absl/random/log_uniform_int_distribution_test.cc b/absl/random/log_uniform_int_distribution_test.cc index 9190f0f19b4..4404111b1f4 100644 --- a/absl/random/log_uniform_int_distribution_test.cc +++ b/absl/random/log_uniform_int_distribution_test.cc @@ -116,31 +116,24 @@ TYPED_TEST(LogUniformIntDistributionTypeTest, SerializeTest) { } } -// A base of 1 (or, for signed types, a negative base) violates the base > 1 -// precondition. In debug builds the constructor asserts; in opt builds it must -// still yield a defined object rather than invoking undefined behavior while -// computing log_range_, which casts 1/log(base) * log(range) to int -- a cast -// of a non-finite double (inf/NaN) to int is UB. The bad base can also reach -// the object through operator>> reading an untrusted stream. -TYPED_TEST(LogUniformIntDistributionTypeTest, InvalidBaseIsDefinedInOptMode) { -#if defined(NDEBUG) - absl::InsecureBitGen gen; +// operator>> must reject input that violates the param_type preconditions +// (max >= min and base > 1) by setting failbit and leaving the distribution +// unchanged, rather than constructing an out-of-contract param_type. +TYPED_TEST(LogUniformIntDistributionTypeTest, DeserializeRejectsInvalidParams) { + for (const char* input : { + "0 100 1", // base == 1 + "0 100 0", // base == 0 + "100 0 2", // max < min + }) { + absl::log_uniform_int_distribution dist(3, 6, 17); + const auto before = dist.param(); - // Direct construction with an out-of-contract base must not invoke UB. - absl::log_uniform_int_distribution dist(0, 100, 1); - auto sample = dist(gen); - EXPECT_GE(sample, dist.min()); - EXPECT_LE(sample, dist.max()); - - // The same bad base arriving through deserialization must also stay defined. - absl::log_uniform_int_distribution after(3, 6, 17); - std::istringstream is("0 100 1"); - is >> after; - EXPECT_EQ(after.base(), static_cast(1)); - sample = after(gen); - EXPECT_GE(sample, after.min()); - EXPECT_LE(sample, after.max()); -#endif // NDEBUG + std::istringstream is(input); + is >> dist; + + EXPECT_TRUE(is.fail()) << input; + EXPECT_EQ(dist.param(), before) << input; + } } using log_uniform_i32 = absl::log_uniform_int_distribution;