Conversation
…sper.cpp A negative FullParams::offset_ms became a negative mel offset in whisper.cpp's encoder (seek_start = offset_ms/10, whisper.cpp:7018), which then read before the start of the mel buffer (2433-2438): an out-of-bounds read reachable from safe code through every transcription path, including 0.1.5. WhisperState::full (used by transcribe*, the streams and the fallback transcriber) and full_parallel now validate offset_ms/duration_ms, and full_parallel also rejects an offset at or past the end of the audio. Sample counts above i32::MAX are rejected instead of being truncated by an `as i32` cast, here and in WhisperVadProcessor.
whisper.cpp indexes the VAD segment vector without a bounds check, so an out-of-range index passed to get_segment_t0/t1 was an out-of-bounds read reachable from safe code. The getters now panic on an out-of-range index, matching the WhisperState result getters.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release notes (draft)
User-facing summary, kept current for reuse in the release notes. Mirrors the
CHANGELOG.mdentries added here.Fixed
offset_ms. A negativeFullParams::offset_msbecame a negative mel offset in whisper.cpp's encoder, which then read before the start of the mel buffer. This was reachable from safe code through every transcription path, including 0.1.5. Transcription now returnsWhisperError::InvalidParameterfor a negativeoffset_msorduration_ms(WhisperState::full, and through ittranscribe*,WhisperStream,WhisperStreamPcmand the temperature-fallback transcriber).WhisperState::full_parallelalso rejects anoffset_msat or past the end of the audio, which whisper.cpp turned into negative chunk sizes.i32::MAXsamples was passed to whisper.cpp with its length silently truncated by anas i32cast.WhisperState::full,full_parallel,WhisperVadProcessor::detect_speechandsegments_from_samplesnow reject it.VadSegments::get_segment_t0()/get_segment_t1()passed the index to whisper.cpp, which does not bounds-check it, so an out-of-range index read outside the segment vector. They now panic on an out-of-range index, like slice indexing and theWhisperStateresult getters.Implementation notes
FullParams::validate()(crate-internal) checks the parameters whisper.cpp uses unchecked;WhisperState::fullandfull_parallelcall it before entering C. Theoffset_mssetter still accepts any value, so the error surfaces at transcription, consistent with other parameter errors.state::sample_count()(crate-internal) replaces theaudio.len() as i32casts with a checked conversion, shared withvad.rs.WhisperStategetter fix: plain-value getters panic on a bad index, like slice indexing. Evidence:whisper_vad_segments_get_segment_t0/t1returnsegments->data[i_segment]with no check (whisper.cpp:5347-5352 at the pinned commit). Raised by the external review of the API-layers design.seek_start = params.offset_ms/10(whisper.cpp:7018) flows intowhisper_encode_internal, wherei0 = std::min(mel_offset, n_len)stays negative andmel_inp.data[j*n_len + i]is read for negativei(whisper.cpp:2433-2438), at the pinned commit.developindependently of feat: whisper.cpp 1.9 API catch-up #15. feat: whisper.cpp 1.9 API catch-up #15 removesWhisperState::full_parallel, so rebasing feat: whisper.cpp 1.9 API catch-up #15 onto this will drop thefull_parallelhunk; the design for feat: whisper.cpp 1.9 API catch-up #15 carries the same validation into the new context-layerfull_parallelandtranscribe_parallel.Validation
Windows / MSVC, test models from
cargo xtask test-setup:cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningsand--features asynccargo test --workspace -- --test-threads=1: 125 passed, none skipped (all model loads used the realggml-tiny.en.bin)cargo test -p whisper-cpp-plus --features async -- --test-threads=1: 112 passedFullParams::validateunit test; integration test that negativeoffset_ms/duration_msare rejected byWhisperState::full,transcribe_with_full_paramsandfull_parallel, thatfull_parallelrejects an offset past the end, and that valid offsets still work; VAD integration test thatget_segment_t0/t1panic for indicesn,n+1,-1,i32::MIN,i32::MAXand for index 0 on a zero-segment result, while valid indices still work.