From 4d73df49455f4dad0c89a4349cbd79dd0aeb4995 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 25 Jul 2026 23:57:49 +0000 Subject: [PATCH] backend(audio-cpp): harden seconds_to_samples against NaN and overflow seconds_to_samples is the one entry point fed by untrusted-shaped input: a float-seconds timestamp off the wire, or a boundary from a model that diverged. Its guard covered only the low side, so NaN and out-of-range values fell through to an undefined double-to-int64 cast and came back as INT64_MIN. A hugely negative sample index used later as an offset or a length is a wild pointer rather than merely a wrong timestamp. Reject NaN with the !(x > 0) form and saturate before the cast. Also round instead of truncating there. These functions exist to cross the float seconds boundary the VAD and diarize messages use, and truncation lost a sample about half the time on the samples-to-seconds-and-back round trip, starting at n=1. Pin the decode scale at INT16_MIN, pin nanosecond truncation on a nonzero fraction, and record why the clamp argument order in f32_to_s16le is load-bearing for NaN. Assisted-by: Claude:claude-opus-5 [Claude Code] Signed-off-by: Ettore Di Giacinto --- backend/cpp/audio-cpp/audio_units.cpp | 39 +++++++++++++-- backend/cpp/audio-cpp/audio_units_test.cpp | 55 ++++++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/backend/cpp/audio-cpp/audio_units.cpp b/backend/cpp/audio-cpp/audio_units.cpp index 6e15cce33..066600e54 100644 --- a/backend/cpp/audio-cpp/audio_units.cpp +++ b/backend/cpp/audio-cpp/audio_units.cpp @@ -2,6 +2,7 @@ #include #include +#include namespace audiocpp_backend { @@ -11,6 +12,8 @@ std::int64_t samples_to_nanoseconds(std::int64_t samples, int sample_rate) { } // Split into whole seconds plus a remainder so the intermediate product // cannot overflow on long recordings, and so rates like 44100 stay exact. + // The remainder division truncates deliberately: that matches Go's + // time.Duration conventions and keeps successive sample indices monotonic. const std::int64_t rate = static_cast(sample_rate); const std::int64_t whole_seconds = samples / rate; const std::int64_t remainder = samples % rate; @@ -26,10 +29,29 @@ float samples_to_seconds(std::int64_t samples, int sample_rate) { } std::int64_t seconds_to_samples(double seconds, int sample_rate) { - if (sample_rate <= 0 || seconds <= 0.0) { + // !(seconds > 0.0) rather than seconds <= 0.0: every comparison against NaN + // is false, so the <= form lets NaN reach the cast below, which is undefined + // behaviour and lands on INT64_MIN in practice. This is the one entry point + // fed by untrusted-shaped input (a float-seconds timestamp off the wire, or + // a boundary from a model that diverged), and a hugely negative sample index + // used later as an offset or a length is a wild pointer rather than merely a + // wrong timestamp. + if (sample_rate <= 0 || !(seconds > 0.0)) { return 0; } - return static_cast(seconds * static_cast(sample_rate)); + const double scaled = seconds * static_cast(sample_rate); + // Bound before the cast for the same reason: converting a double at or above + // 2^63 (infinity included) is undefined behaviour, so saturate instead. + const double limit = + static_cast(std::numeric_limits::max()); + if (scaled >= limit) { + return std::numeric_limits::max(); + } + // Round rather than truncate: these functions exist to cross the float + // seconds boundary the VAD and diarize messages use, so a value that came + // from samples_to_seconds must convert back to the sample it started as. + // Truncation loses one sample about half the time, starting at n=1. + return static_cast(std::llround(scaled)); } std::vector s16le_to_f32(const std::string &bytes) { @@ -42,6 +64,12 @@ std::vector s16le_to_f32(const std::string &bytes) { const auto raw = static_cast( static_cast(low) | (static_cast(high) << 8)); + // 32768 on decode against 32767 on encode is deliberate, not a typo. + // 32768 is what keeps INT16_MIN at exactly -1.0 and every other code + // inside the [-1, 1] range this header promises; dividing by 32767 + // would decode INT16_MIN to -1.00003. See f32_to_s16le for the other + // half of the pair. The cost is that a round trip shrinks a sample by + // 32767/32768, well under one LSB. samples.push_back(static_cast(raw) / 32768.0f); } return samples; @@ -51,9 +79,14 @@ std::string f32_to_s16le(const std::vector &samples) { std::string bytes; bytes.reserve(samples.size() * 2); for (const float sample : samples) { + // Argument order is load-bearing: std::min(1.0f, sample) returns 1.0f + // for a NaN sample, because NaN < 1.0f is false and min returns its + // first argument in that case. Written the equally natural + // std::min(sample, 1.0f), a NaN would pass straight through to + // std::lround, whose result is unspecified for NaN. Do not reorder. const float clamped = std::max(-1.0f, std::min(1.0f, sample)); // 32767 rather than 32768 so +1.0 saturates at INT16_MAX instead of - // overflowing to INT16_MIN. + // overflowing to INT16_MIN. See s16le_to_f32 for why decode differs. const auto value = static_cast(std::lround(clamped * 32767.0f)); const auto raw = static_cast(value); diff --git a/backend/cpp/audio-cpp/audio_units_test.cpp b/backend/cpp/audio-cpp/audio_units_test.cpp index 3b56911a0..ff7acd744 100644 --- a/backend/cpp/audio-cpp/audio_units_test.cpp +++ b/backend/cpp/audio-cpp/audio_units_test.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -49,6 +50,11 @@ static void test_nanoseconds() { // truncates this one a nanosecond short. Integer division does not. check(samples_to_nanoseconds(4004, 8000) == 500500000LL, "0.5005s at 8k is exact to the nanosecond"); + // Truncation, not rounding: this matches Go's time.Duration conventions and + // keeps successive sample indices monotonic. The exact value here is + // 22675.7...; rounding to nearest would give 22676. + check(samples_to_nanoseconds(1, 44100) == 22675LL, + "a sub-nanosecond fraction truncates rather than rounding up"); } static void test_seconds() { @@ -59,6 +65,31 @@ static void test_seconds() { check(seconds_to_samples(0.5, 16000) == 8000, "0.5s to samples at 16k"); check(seconds_to_samples(1.0, 0) == 0, "zero sample rate yields zero samples"); check(seconds_to_samples(-1.0, 16000) == 0, "negative seconds clamps to zero"); + + // seconds_to_samples is the one entry point fed by untrusted-shaped input: + // a float-seconds timestamp off the wire, or a VAD boundary from a model + // that diverged. A hugely negative sample index used later as an offset or + // a length is a wild pointer, not merely a wrong timestamp. + const double nan_seconds = std::numeric_limits::quiet_NaN(); + const double inf_seconds = std::numeric_limits::infinity(); + const std::int64_t max_samples = std::numeric_limits::max(); + check(seconds_to_samples(nan_seconds, 16000) == 0, "NaN seconds yields zero"); + check(seconds_to_samples(inf_seconds, 16000) == max_samples, + "infinite seconds saturates instead of overflowing"); + check(seconds_to_samples(1e30, 16000) == max_samples, + "out of range seconds saturates instead of overflowing"); + check(seconds_to_samples(-inf_seconds, 16000) == 0, + "negative infinity clamps to zero"); + + // Crossing the float-seconds boundary and back is the expected round trip + // for the VAD and diarize messages, so it must not lose a sample. + // Truncation loses one about half the time, starting at n=1. + check(seconds_to_samples(samples_to_seconds(1, 44100), 44100) == 1, + "one sample survives the seconds round trip at 44.1k"); + check(seconds_to_samples(samples_to_seconds(1, 16000), 16000) == 1, + "one sample survives the seconds round trip at 16k"); + check(seconds_to_samples(samples_to_seconds(4001, 8000), 8000) == 4001, + "4001 samples survive the seconds round trip at 8k"); } static void test_s16le_round_trip() { @@ -95,6 +126,28 @@ static void test_s16le_clamping() { "negative overshoot clamps to full scale"); } +static void test_s16le_decode_range() { + // INT16_MIN is the one value that pins the decode scale. Dividing by 32767 + // instead of 32768 would decode it to -1.00003, outside the [-1, 1] range + // the header promises, and every other test would still pass. + const std::vector decoded = s16le_to_f32(std::string("\x00\x80", 2)); + check(decoded.size() == 1, "INT16_MIN decodes to one sample"); + check(decoded.size() == 1 && decoded[0] == -1.0f, + "INT16_MIN decodes to exactly -1.0, not past full scale"); +} + +static void test_s16le_nan_input() { + // A NaN sample must not reach std::lround, whose result is unspecified for + // NaN. See the argument-order comment in f32_to_s16le. + const std::vector decoded = + s16le_to_f32(f32_to_s16le({std::numeric_limits::quiet_NaN()})); + check(decoded.size() == 1, "a NaN sample still encodes to one sample"); + check(decoded.size() == 1 && std::isfinite(decoded[0]), + "a NaN sample encodes to a finite value"); + check(decoded.size() == 1 && decoded[0] >= -1.0f && decoded[0] <= 1.0f, + "a NaN sample encodes within full scale"); +} + static void test_s16le_odd_length() { // A truncated frame must drop the dangling byte rather than read past it. const std::string odd(5, '\0'); @@ -108,6 +161,8 @@ int main() { test_s16le_round_trip(); test_s16le_endianness(); test_s16le_clamping(); + test_s16le_decode_range(); + test_s16le_nan_input(); test_s16le_odd_length(); if (failures) { fprintf(stderr, "%d check(s) failed\n", failures);