diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bd5be9..1fe1fa6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed a use-after-free in `FullParams::suppress_regex()`: the regex string was freed immediately after being set, so whisper.cpp read freed memory during transcription. - Fixed `FullParams::prompt_tokens()` storing a borrowed pointer that dangled once the caller's slice was dropped or the params were moved or cloned. The tokens are now copied into the params. - **Breaking:** `WhisperState` result getters now validate segment and token indices instead of passing them to whisper.cpp, which does not bounds-check (out-of-range indices were undefined behaviour). `full_get_segment_text()` and `full_get_token_text()` return `WhisperError::InvalidParameter`, `full_get_token_data()` returns `None`, and the plain-value getters (`full_get_segment_timestamps()`, `full_get_segment_speaker_turn_next()`, `full_n_tokens()`, `full_get_token_id()`, `full_get_token_prob()`) panic, like slice indexing. +- Fixed an out-of-bounds read in whisper.cpp reachable from safe code: a negative `FullParams::offset_ms` became a negative mel offset in the encoder, which then read before the start of the mel buffer. Transcription (`WhisperState::full`, and through it `transcribe*`, `WhisperStream`, `WhisperStreamPcm` and the temperature-fallback transcriber) now returns `WhisperError::InvalidParameter` for a negative `offset_ms` or `duration_ms`. `WhisperState::full_parallel` also rejects an `offset_ms` at or past the end of the audio, which whisper.cpp turned into negative chunk sizes. +- Audio with more than `i32::MAX` samples is now rejected instead of having its length silently truncated when passed to whisper.cpp (`WhisperState::full`, `full_parallel`, `WhisperVadProcessor::detect_speech` / `segments_from_samples`). +- Fixed an out-of-bounds read in whisper.cpp reachable from safe code: `VadSegments::get_segment_t0()` / `get_segment_t1()` passed the segment index to whisper.cpp, which does not bounds-check it. They now panic on an out-of-range index, like slice indexing and the `WhisperState` result getters. ## [0.1.5] - 2026-06-12 diff --git a/whisper-cpp-plus/src/params.rs b/whisper-cpp-plus/src/params.rs index 62add32..fe6709b 100644 --- a/whisper-cpp-plus/src/params.rs +++ b/whisper-cpp-plus/src/params.rs @@ -1,3 +1,4 @@ +use crate::error::{Result, WhisperError}; use std::ffi::CString; use whisper_cpp_plus_sys as ffi; @@ -61,6 +62,26 @@ impl FullParams { params } + /// Rejects parameter values that whisper.cpp uses without checking. + /// + /// A negative `offset_ms` becomes a negative mel offset in whisper.cpp's encoder, which then + /// reads before the start of the mel buffer. + pub(crate) fn validate(&self) -> Result<()> { + if self.inner.offset_ms < 0 { + return Err(WhisperError::InvalidParameter(format!( + "offset_ms must not be negative (got {})", + self.inner.offset_ms + ))); + } + if self.inner.duration_ms < 0 { + return Err(WhisperError::InvalidParameter(format!( + "duration_ms must not be negative (got {})", + self.inner.duration_ms + ))); + } + Ok(()) + } + pub(crate) fn as_raw(&self) -> ffi::whisper_full_params { let mut params = self.inner; @@ -386,4 +407,23 @@ mod tests { assert!(raw.prompt_tokens.is_null()); assert_eq!(raw.prompt_n_tokens, 0); } + + #[test] + fn validate_rejects_negative_offset_and_duration() { + assert!(FullParams::default().validate().is_ok()); + assert!(FullParams::default() + .offset_ms(1000) + .duration_ms(500) + .validate() + .is_ok()); + + assert!(matches!( + FullParams::default().offset_ms(-1).validate(), + Err(WhisperError::InvalidParameter(_)) + )); + assert!(matches!( + FullParams::default().duration_ms(-10).validate(), + Err(WhisperError::InvalidParameter(_)) + )); + } } diff --git a/whisper-cpp-plus/src/state.rs b/whisper-cpp-plus/src/state.rs index a95bf11..2e9e59a 100644 --- a/whisper-cpp-plus/src/state.rs +++ b/whisper-cpp-plus/src/state.rs @@ -4,6 +4,20 @@ use crate::params::FullParams; use std::sync::Arc; use whisper_cpp_plus_sys as ffi; +/// Returns the sample count whisper.cpp takes as an `int`, rejecting empty or oversized audio. +pub(crate) fn sample_count(audio: &[f32]) -> Result { + if audio.is_empty() { + return Err(WhisperError::InvalidAudioFormat); + } + i32::try_from(audio.len()).map_err(|_| { + WhisperError::InvalidParameter(format!( + "audio has {} samples; whisper.cpp accepts at most {}", + audio.len(), + i32::MAX + )) + }) +} + pub struct WhisperState { pub(crate) ptr: *mut ffi::whisper_state, pub(crate) _context: Arc, @@ -34,9 +48,8 @@ impl WhisperState { } pub fn full(&mut self, params: FullParams, audio: &[f32]) -> Result<()> { - if audio.is_empty() { - return Err(WhisperError::InvalidAudioFormat); - } + let n_samples = sample_count(audio)?; + params.validate()?; let ret = unsafe { ffi::whisper_full_with_state( @@ -44,7 +57,7 @@ impl WhisperState { self.ptr, params.as_raw(), audio.as_ptr(), - audio.len() as i32, + n_samples, ) }; @@ -64,9 +77,8 @@ impl WhisperState { audio: &[f32], n_processors: i32, ) -> Result<()> { - if audio.is_empty() { - return Err(WhisperError::InvalidAudioFormat); - } + let n_samples = sample_count(audio)?; + params.validate()?; if n_processors < 1 { return Err(WhisperError::InvalidParameter( @@ -74,12 +86,22 @@ impl WhisperState { )); } + // whisper_full_parallel splits the audio after the offset without checking it; an offset at + // or past the end gives negative per-chunk sample counts. + let offset_samples = + i64::from(params.inner.offset_ms) * i64::from(ffi::WHISPER_SAMPLE_RATE) / 1000; + if offset_samples >= i64::from(n_samples) { + return Err(WhisperError::InvalidParameter( + "offset_ms must be within the audio".into(), + )); + } + let ret = unsafe { ffi::whisper_full_parallel( self._context.0, params.as_raw(), audio.as_ptr(), - audio.len() as i32, + n_samples, n_processors, ) }; diff --git a/whisper-cpp-plus/src/vad.rs b/whisper-cpp-plus/src/vad.rs index ea5cbab..0fc6052 100644 --- a/whisper-cpp-plus/src/vad.rs +++ b/whisper-cpp-plus/src/vad.rs @@ -4,6 +4,7 @@ //! in audio before transcription, improving performance and accuracy. use crate::error::{Result, WhisperError}; +use crate::state::sample_count; use std::path::Path; use whisper_cpp_plus_sys as ffi; @@ -138,12 +139,14 @@ impl WhisperVadProcessor { } /// Detect speech in audio samples + /// + /// Returns `false` without calling whisper.cpp for empty input or more than `i32::MAX` samples. pub fn detect_speech(&mut self, samples: &[f32]) -> bool { - if samples.is_empty() { + let Ok(n_samples) = sample_count(samples) else { return false; - } + }; - unsafe { ffi::whisper_vad_detect_speech(self.ctx, samples.as_ptr(), samples.len() as i32) } + unsafe { ffi::whisper_vad_detect_speech(self.ctx, samples.as_ptr(), n_samples) } } /// Get the number of probability values @@ -185,16 +188,14 @@ impl WhisperVadProcessor { samples: &[f32], params: &VadParams, ) -> Result { - if samples.is_empty() { - return Err(WhisperError::InvalidAudioFormat); - } + let n_samples = sample_count(samples)?; let segments_ptr = unsafe { ffi::whisper_vad_segments_from_samples( self.ctx, params.to_ffi(), samples.as_ptr(), - samples.len() as i32, + n_samples, ) }; @@ -227,14 +228,33 @@ impl VadSegments { unsafe { ffi::whisper_vad_segments_n_segments(self.ptr) } } + // whisper.cpp indexes the segment vector without a bounds check. + fn assert_segment_in_range(&self, i_segment: i32) { + assert!( + (0..self.n_segments()).contains(&i_segment), + "VAD segment index {} out of range", + i_segment + ); + } + /// Get segment start time in seconds + /// + /// # Panics + /// + /// Panics if `i_segment` is out of range. pub fn get_segment_t0(&self, i_segment: i32) -> f32 { + self.assert_segment_in_range(i_segment); // The FFI returns time in centiseconds, convert to seconds unsafe { ffi::whisper_vad_segments_get_segment_t0(self.ptr, i_segment) / 100.0 } } /// Get segment end time in seconds + /// + /// # Panics + /// + /// Panics if `i_segment` is out of range. pub fn get_segment_t1(&self, i_segment: i32) -> f32 { + self.assert_segment_in_range(i_segment); // The FFI returns time in centiseconds, convert to seconds unsafe { ffi::whisper_vad_segments_get_segment_t1(self.ptr, i_segment) / 100.0 } } diff --git a/whisper-cpp-plus/tests/integration.rs b/whisper-cpp-plus/tests/integration.rs index a1412cf..4a5a316 100644 --- a/whisper-cpp-plus/tests/integration.rs +++ b/whisper-cpp-plus/tests/integration.rs @@ -222,3 +222,46 @@ fn test_empty_audio_error() { _ => panic!("Expected InvalidAudioFormat error"), } } + +#[test] +fn test_rejects_invalid_offset_and_duration() { + let Some(model_path) = TestModels::tiny_en() else { + eprintln!("Skipping: model not found. Run `cargo xtask test-setup`"); + return; + }; + + let ctx = WhisperContext::new(&model_path).unwrap(); + let audio = vec![0.0f32; 16000]; + let params = || FullParams::new(SamplingStrategy::Greedy { best_of: 1 }); + let is_invalid_parameter = |result: whisper_cpp_plus::Result<()>| { + matches!(result, Err(WhisperError::InvalidParameter(_))) + }; + + // A negative offset used to reach an out-of-bounds read in whisper.cpp's encoder. + let mut state = ctx.create_state().unwrap(); + assert!(is_invalid_parameter( + state.full(params().offset_ms(-1000), &audio) + )); + assert!(is_invalid_parameter( + state.full(params().duration_ms(-1), &audio) + )); + assert!(matches!( + ctx.transcribe_with_full_params(&audio, params().offset_ms(-1)), + Err(WhisperError::InvalidParameter(_)) + )); + + // full_parallel additionally requires the offset to fall within the audio. + assert!(is_invalid_parameter(state.full_parallel( + params().offset_ms(-1), + &audio, + 2 + ))); + assert!(is_invalid_parameter(state.full_parallel( + params().offset_ms(1000), + &audio, + 2 + ))); + + // Valid offsets still work. + assert!(state.full(params().offset_ms(100), &audio).is_ok()); +} diff --git a/whisper-cpp-plus/tests/vad_integration.rs b/whisper-cpp-plus/tests/vad_integration.rs index d6958c5..56acafe 100644 --- a/whisper-cpp-plus/tests/vad_integration.rs +++ b/whisper-cpp-plus/tests/vad_integration.rs @@ -368,6 +368,66 @@ fn test_vad_with_silence() { println!("✓ VAD correctly detected no speech in silence"); } +#[test] +fn test_vad_segment_getters_reject_out_of_range_indices() { + // whisper.cpp does not bounds-check VAD segment indices; the safe getters must. + let vad_model_path = find_vad_model(); + let jfk_path = find_jfk_audio(); + + if vad_model_path.is_none() { + eprintln!("Skipping: VAD model not found. Set WHISPER_TEST_MODEL_DIR or run `cargo xtask test-setup`"); + return; + } + if jfk_path.is_none() { + eprintln!("Skipping: JFK audio not found. Set WHISPER_TEST_AUDIO_DIR or run `cargo xtask test-setup`"); + return; + } + + let mut vad = + WhisperVadProcessor::new(vad_model_path.unwrap()).expect("Failed to load VAD model"); + let audio = load_wav_16khz_mono(&jfk_path.unwrap()).expect("Failed to load JFK audio"); + let silence = vec![0.0f32; 16000 * 3]; + let vad_params = VadParams::default(); + + let speech = vad + .segments_from_samples(&audio, &vad_params) + .expect("Failed to detect speech segments"); + let n = speech.n_segments(); + assert!(n > 0, "Should detect speech in JFK audio"); + + // Valid indices still work. + let last = n - 1; + assert!(speech.get_segment_t1(last) > speech.get_segment_t0(last)); + + let empty = vad + .segments_from_samples(&silence, &vad_params) + .expect("Failed to process silence"); + assert_eq!(empty.n_segments(), 0); + + let panics = + |f: &dyn Fn() -> f32| std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)).is_err(); + for i in [n, n + 1, -1, i32::MIN, i32::MAX] { + assert!( + panics(&|| speech.get_segment_t0(i)), + "t0({}) should panic", + i + ); + assert!( + panics(&|| speech.get_segment_t1(i)), + "t1({}) should panic", + i + ); + } + assert!( + panics(&|| empty.get_segment_t0(0)), + "t0(0) on no segments should panic" + ); + assert!( + panics(&|| empty.get_segment_t1(0)), + "t1(0) on no segments should panic" + ); +} + #[test] fn test_vad_with_mixed_audio() { // Test VAD with artificially created mixed audio (speech-like noise + silence)