Skip to content

Commit 15fd4e3

Browse files
committed
Fixed several instances of UB in floating point arithmetic, as pointed out in Issue microsoft#732. Thank you @Wowblk.
1 parent f0e4eb7 commit 15fd4e3

21 files changed

Lines changed: 141 additions & 95 deletions

CHANGES.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44

55
- Merged [(PR #734)](https://github.com/microsoft/SEAL/pull/734): Fixed `IterTuple` constructors that returned references to local temporaries.
66
- Merged [(PR #738)](https://github.com/microsoft/SEAL/pull/738): Fixed out-of-bounds read in `Serialization_IsCompatibleVersion` and `Serialization_IsValidHeader` (C API).
7+
- Fixed undefined behavior in CKKS encoding reported in [(issue #732)](https://github.com/microsoft/SEAL/issues/732) by [Wowblk](https://github.com/Wowblk), and other instances of a similar broken pattern.
78
- Bumped .NET target framework from `net8.0` to `net10.0`.
9+
- Updated dependency versions: Google Benchmark to 1.9.5, GoogleTest to 1.17.0, MSGSL to 4.2.1, zlib to 1.3.2.
810
- Added `cmake/ios_xcframework.cmake` and a CI artifact that produces `libseal-<ver>.xcframework` and `libsealc-<ver>.xcframework` in one command.
911
- Added `CMakePresets.json` for Windows, Linux, and macOS development.
1012
- No further releases will be published to NuGet.org. Users who want newer versions in .NET projects should build their own NuGet package from source.

cmake/ExternalBenchmark.cmake

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
FetchContent_Declare(
55
benchmark
66
GIT_REPOSITORY https://github.com/google/benchmark.git
7-
GIT_TAG afa23b7699c17f1e26c88cbf95257b20d78d6247 # 1.9.2
7+
GIT_TAG 192ef10025eb2c4cdd392bc502f0c852196baa48 # 1.9.5
88
)
99
FetchContent_GetProperties(benchmark)
1010

cmake/ExternalGTest.cmake

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
FetchContent_Declare(
55
googletest
66
GIT_REPOSITORY https://github.com/google/googletest.git
7-
GIT_TAG 6910c9d9165801d8827d628cb72eb7ea9dd538c5 # 1.16.0
7+
GIT_TAG 52eb8108c5bdec04579160ae17225d66034bd723 # 1.17.0
88
)
99
FetchContent_GetProperties(googletest)
1010

cmake/ExternalMSGSL.cmake

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
FetchContent_Declare(
55
msgsl
66
GIT_REPOSITORY https://github.com/microsoft/GSL.git
7-
GIT_TAG 2828399820ef4928cc89b65605dca5dc68efca6e # 4.2.0
7+
GIT_TAG 249146590e322123cd0b378b1f364d6069717687 # 4.2.1
88
)
99
FetchContent_GetProperties(msgsl)
1010

cmake/ExternalZLIB.cmake

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
FetchContent_Declare(
44
zlib
55
GIT_REPOSITORY https://github.com/madler/zlib.git
6-
GIT_TAG 51b7f2abdade71cd9bb0e7a373ef2610ec6f9daf # 1.3.1
6+
GIT_TAG da607da739fa6047df13e66a2af6b8bec7c2a498 # 1.3.2
77
)
88
FetchContent_GetProperties(zlib)
99

native/src/seal/c/defines.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ static_assert(false, "Require architecture == x64");
4848

4949
#define FACILITY_WIN32 7
5050
#define HRESULT_FROM_WIN32(x) \
51-
((HRESULT)(x) <= 0 ? ((HRESULT)(x)) : ((HRESULT)(((x)&0x0000FFFF) | (FACILITY_WIN32 << 16) | 0x80000000)))
51+
((HRESULT)(x) <= 0 ? ((HRESULT)(x)) : ((HRESULT)(((x) & 0x0000FFFF) | (FACILITY_WIN32 << 16) | 0x80000000)))
5252

5353
#define ERROR_INSUFFICIENT_BUFFER 122L
5454
#define ERROR_INVALID_INDEX 1413L

native/src/seal/ckks.cpp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,15 +99,31 @@ namespace seal
9999
}
100100

101101
// Check that scale is positive and not too large
102-
if (scale <= 0 || (static_cast<int>(log2(scale)) >= context_data.total_coeff_modulus_bit_count()))
102+
if (!isfinite(scale) || scale <= 0 ||
103+
(static_cast<int>(log2(scale)) >= context_data.total_coeff_modulus_bit_count()))
103104
{
104105
throw invalid_argument("scale out of bounds");
105106
}
106107

108+
// Reject non-finite user input up front.
109+
if (!isfinite(value))
110+
{
111+
throw invalid_argument("value must be finite");
112+
}
113+
107114
// Compute the scaled value
108115
value *= scale;
109116

110-
int coeff_bit_count = static_cast<int>(log2(fabs(value))) + 2;
117+
// Catch overflow in value * scale before the cast: even finite
118+
// operands can produce inf here.
119+
if (!isfinite(value))
120+
{
121+
throw invalid_argument("encoded value is too large");
122+
}
123+
124+
// Guard against fabs(value) < 1 (log2 negative or -inf -> UB on cast).
125+
// Anything with magnitude below 1 fits trivially in the fast path.
126+
int coeff_bit_count = (fabs(value) < 1.0) ? 2 : (static_cast<int>(log2(fabs(value))) + 2);
111127
if (coeff_bit_count >= context_data.total_coeff_modulus_bit_count())
112128
{
113129
throw invalid_argument("encoded value is too large");

native/src/seal/ckks.h

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -481,11 +481,32 @@ namespace seal
481481
}
482482

483483
// Check that scale is positive and not too large
484-
if (scale <= 0 || (static_cast<int>(log2(scale)) + 1 >= context_data.total_coeff_modulus_bit_count()))
484+
if (!std::isfinite(scale) || scale <= 0 ||
485+
(static_cast<int>(log2(scale)) + 1 >= context_data.total_coeff_modulus_bit_count()))
485486
{
486487
throw std::invalid_argument("scale out of bounds");
487488
}
488489

490+
// Reject non-finite user inputs up front so they can't drive the
491+
// FFT into producing inf/NaN coefficients.
492+
for (std::size_t i = 0; i < values_size; i++)
493+
{
494+
if constexpr (std::is_same<std::remove_cv_t<T>, double>::value)
495+
{
496+
if (!std::isfinite(values[i]))
497+
{
498+
throw std::invalid_argument("values must be finite");
499+
}
500+
}
501+
else
502+
{
503+
if (!std::isfinite(values[i].real()) || !std::isfinite(values[i].imag()))
504+
{
505+
throw std::invalid_argument("values must be finite");
506+
}
507+
}
508+
}
509+
489510
auto ntt_tables = context_data.small_ntt_tables();
490511

491512
// values_size is guaranteed to be no bigger than slots_
@@ -506,10 +527,16 @@ namespace seal
506527
{
507528
max_coeff = std::max<>(max_coeff, std::fabs(conj_values[i].real()));
508529
}
530+
// Catch FFT overflow before the cast: even finite inputs can
531+
// produce inf coefficients after multiplication by scale/n.
532+
if (!std::isfinite(max_coeff))
533+
{
534+
throw std::invalid_argument("encoded values are too large");
535+
}
509536
// Verify that the values are not too large to fit in coeff_modulus
510537
// Note that we have an extra + 1 for the sign bit
511538
// Don't compute logarithmis of numbers less than 1
512-
int max_coeff_bit_count = static_cast<int>(std::ceil(std::log2(std::max<>(max_coeff, 1.0)))) + 1;
539+
int max_coeff_bit_count = util::safe_ceil_log2_int(std::max<>(max_coeff, 1.0)) + 1;
513540
if (max_coeff_bit_count >= context_data.total_coeff_modulus_bit_count())
514541
{
515542
throw std::invalid_argument("encoded values are too large");
@@ -661,7 +688,7 @@ namespace seal
661688
auto ntt_tables = context_data.small_ntt_tables();
662689

663690
// Check that scale is positive and not too large
664-
if (plain.scale() <= 0 ||
691+
if (!std::isfinite(plain.scale()) || plain.scale() <= 0 ||
665692
(static_cast<int>(log2(plain.scale())) >= context_data.total_coeff_modulus_bit_count()))
666693
{
667694
throw std::invalid_argument("scale out of bounds");

native/src/seal/evaluator.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ namespace seal
4444
scale_bit_count_bound = -1;
4545
};
4646

47-
return !(scale <= 0 || (static_cast<int>(log2(scale)) >= scale_bit_count_bound));
47+
return !(!isfinite(scale) || scale <= 0 || (static_cast<int>(log2(scale)) >= scale_bit_count_bound));
4848
}
4949

5050
/**

native/src/seal/memorymanager.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ namespace seal
483483
instructions to the memory manager profile for internal logic.
484484
*/
485485
template <typename... Args>
486-
SEAL_NODISCARD static inline MemoryPoolHandle GetPool(mm_prof_opt_t prof_opt, Args &&... args)
486+
SEAL_NODISCARD static inline MemoryPoolHandle GetPool(mm_prof_opt_t prof_opt, Args &&...args)
487487
{
488488
switch (prof_opt)
489489
{

0 commit comments

Comments
 (0)