Harden language model readers against malformed input - #499
Open
lenzo-ka wants to merge 2 commits into
Open
Conversation
pocketsphinx_lm_convert reads untrusted language model files, and several reader paths crashed on crafted input rather than failing cleanly. Each of the following now rejects the malformed input with a diagnostic and a NULL return: - read_dmp_weight_array (ngrams_raw.c) read a weight-array size and per n-gram weight indices from the file and used them without validation. Check the reads, reject a non-positive size, reject a size that cannot fit in the remaining file before allocating, and bounds-check each index against the array; propagate failure through the DMP reader. - ngrams_raw_free (ngrams_raw.c) dereferenced per-order sub-arrays that are NULL when an earlier order failed to build. Guard the array and each sub-array before freeing. - ngrams_raw_read_arpa (ngrams_raw.c) dereferenced the line iterator when an ARPA file ended exactly at the end-mark position. Check for NULL and warn instead. - ngram_model_trie_read_dmp (ngram_model_trie.c) passed a format string with two conversions but one argument when reporting a bad header, reading an argument that was never supplied. Report the file name once with a single conversion. - ngram_model_trie_read_dmp ignored read_word_str's return value, so a truncated word-string block left word_str entries NULL and a later consumer dereferenced them. Check the return value and fail cleanly, matching the binary reader. Behavior on well-formed models is unchanged.
Construct minimal malformed DMP and ARPA models in the test process and drive each through ngram_model_read, asserting a clean failure without a crash. The constructions cover the oversized DMP weight-array count, the partially built n-gram array freed on an incomplete ARPA section, the ARPA model with no end-mark, and the DMP model with a truncated word-string block. Each fails against the previous reader implementation.
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.
pocketsphinx_lm_convertreads untrusted language model files, and several reader paths crashed on crafted input rather than failing cleanly. Each of the following now rejects the malformed input with a diagnostic and a NULL return, while well-formed models load exactly as before:read_dmp_weight_array(ngrams_raw.c) read a weight-array size and per–n-gram weight indices from the file and used them without validation. It now checks the read, rejects a non-positive size, rejects a size that cannot fit in the remaining file before allocating, and bounds-checks each index against the array; failure is propagated through the DMP reader.ngrams_raw_free(ngrams_raw.c) dereferenced per-order sub-arrays that are NULL when an earlier order failed to build. It now guards the array and each sub-array before freeing.ngrams_raw_read_arpa(ngrams_raw.c) dereferenced the line iterator when an ARPA file ended exactly at the end-mark position. It now checks for NULL and warns instead.ngram_model_trie_read_dmp(ngram_model_trie.c) passed a format string with two conversions but one argument when reporting a bad header, reading an argument that was never supplied. It now reports the file name once with a single conversion.ngram_model_trie_read_dmpignoredread_word_str's return value, so a truncated word-string block leftword_strentries NULL and a later consumer dereferenced them. It now checks the return value and fails cleanly, matching the binary reader.A new unit test,
test_lm_reader_hardening, constructs minimal malformed DMP and ARPA models in the test process and drives each throughngram_model_read, asserting a clean failure without a crash: an invalid weight-array count, a weight-array count larger than the remaining file, a partially built n-gram array freed on an incomplete ARPA section, an ARPA model with no end-mark, and a DMP model with a truncated word-string block. Each case fails against the previous reader implementation.Reported in #482. The five constructions there reproduce the crashes under AddressSanitizer; the shipped
en-usandtidigitsmodels and the DMP fixtures, including a byte-swapped DMP, continue to load with these changes.Fixes #482
Tested on macOS (Apple clang) and Linux x86-64 (GCC 13.3); full unit and regression suite passes on both.