Fix integer overflow in LSTM Convolve and Reconfig deserialization - #4588
Merged
Merged
Conversation
Add range and overflow validation in Convolve::DeSerialize and Reconfig::DeSerialize to prevent a crafted .traineddata file from triggering a heap out-of-bounds write via unchecked signed integer multiplication when computing the output-channel count. Validate ni/no/num_weights in Network::CreateFromFile. Add defense-in-depth bounds assertions in NetworkIO::Randomize and NetworkIO::CopyTimeStepGeneral. Signed-off-by: Stefan Weil <sw@weilnetz.de> Assisted-by: OpenCode / big-pickle (opencode)
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
src/lstm/networkio.cpp:412
- The new bounds assert is incomplete and can still allow out-of-bounds memory access: negative
dest_offset/src_offsetornum_featureswill passdest_offset + num_features <= NumFeatures(), and the addition itself can overflow. Also, there is no corresponding bounds assertion for the source range. Use non-negative checks and subtraction-based bounds (and validate both dest and src) to avoid overflow and underflow.
This issue also appears on line 417 of the same file.
void NetworkIO::CopyTimeStepGeneral(int dest_t, int dest_offset, int num_features,
const NetworkIO &src, int src_t, int src_offset) {
ASSERT_HOST(int_mode_ == src.int_mode_);
ASSERT_HOST(dest_offset + num_features <= NumFeatures());
if (int_mode_) {
memcpy(i_[dest_t] + dest_offset, src.i_[src_t] + src_offset, num_features * sizeof(i_[0][0]));
} else {
memcpy(f_[dest_t] + dest_offset, src.f_[src_t] + src_offset, num_features * sizeof(f_[0][0]));
src/lstm/networkio.cpp:420
- The new bounds assert in Randomize can still allow out-of-bounds writes when
offsetornum_featuresis negative, andoffset + num_featurescan overflow. Prefer non-negative checks and subtraction-based bounds to avoid overflow/underflow.
void NetworkIO::Randomize(int t, int offset, int num_features, TRand *randomizer) {
ASSERT_HOST(offset + num_features <= NumFeatures());
if (int_mode_) {
int8_t *line = i_[t] + offset;
Comment on lines
+72
to
+78
| int64_t product = static_cast<int64_t>(ni_) * x_scale_ * y_scale_; | ||
| if (product > INT_MAX) { | ||
| tprintf("Error: Reconfig output-channel count overflows: ni=%d x_scale=%d y_scale=%d\n", ni_, | ||
| x_scale_, y_scale_); | ||
| return false; | ||
| } | ||
| no_ = static_cast<int>(product); |
Comment on lines
+57
to
+63
| int64_t product = static_cast<int64_t>(ni_) * (2LL * half_x_ + 1) * (2LL * half_y_ + 1); | ||
| if (product > INT_MAX) { | ||
| tprintf("Error: Convolve output-channel count overflows: ni=%d half_x=%d half_y=%d\n", ni_, | ||
| half_x_, half_y_); | ||
| return false; | ||
| } | ||
| no_ = static_cast<int>(product); |
The previous int64_t widening multiplication could itself overflow with extreme file-controlled values, which is undefined behavior and could bypass validation. Replace with division-based bounds checks so no intermediate product ever exceeds INT_MAX. Signed-off-by: Stefan Weil <sw@weilnetz.de> Assisted-by: OpenCode / big-pickle (opencode)
Member
Author
|
@EunhoKim98, please review and test if possible. |
|
"Thanks Stefan — I reviewed the fix and it looks solid. It covers everything raised in the report. PR #4588 and version 5.5.3 look good to go. Thanks again for the quick turnaround — please feel free to request the CVE when you're ready." |
social4hyq
pushed a commit
to social4hyq/homebrew-core
that referenced
this pull request
Sep 20, 2026
tesseract 5.5.3 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`. - [ ] `resource` blocks have been checked for updates. <details> <summary>release notes</summary> <pre>## What's Changed * Fix typos by @szepeviktor in tesseract-ocr/tesseract#4497 * Fix missing closing tags in multi-page PAGE XML output by @stweil with @Copilot in tesseract-ocr/tesseract#4506 * Fix Apple CMAKE_SYSTEM_PROCESSOR not set when crosscompiling by @ppavacic in tesseract-ocr/tesseract#4503 * Remove outdated docker-compose.yml (Fixes #4492) by @Mohataseem89 in tesseract-ocr/tesseract#4509 * docs: document memory ownership and lifecycle in C-API by @markbus-ai in tesseract-ocr/tesseract#4511 * Add Portuguese language support and section descriptions (Windows installer) by @eduardomozart in tesseract-ocr/tesseract#4516 * Update Slovak language string in tesseract.nsi by @eduardomozart in tesseract-ocr/tesseract#4518 * fix(nsi): mask LANGID to 16-bit for reliable auto-detection by @eduardomozart in tesseract-ocr/tesseract#4513 * Add GitHub Copilot instructions by @stweil with @Copilot in tesseract-ocr/tesseract#4508 * Correct mutex call to prevent multiple instances by @eduardomozart in tesseract-ocr/tesseract#4517 * Update versions of GitHub actions by @stweil in tesseract-ocr/tesseract#4522 * Bump microsoft/setup-msbuild from 2 to 3 by @dependabot[bot] in tesseract-ocr/tesseract#4533 * Use asciidoctor instead of asciidoc-py for manpage generation by @amitdo with @Copilot in tesseract-ocr/tesseract#4534 * Bump actions/upload-artifact from 4 to 7 by @dependabot[bot] in tesseract-ocr/tesseract#4545 * autotools: Fix linker warning on macOS by @stweil in tesseract-ocr/tesseract#4559 * Fix compiler warning on macOS by @stweil in tesseract-ocr/tesseract#4558 * Cmake cleanup by @zdenop in tesseract-ocr/tesseract#4562 * Remove SW_BUILD option from CMake and CI by @amitdo with @Copilot in tesseract-ocr/tesseract#4566 * Modernize code by @stweil in tesseract-ocr/tesseract#4561 * Remove some files by @stweil in tesseract-ocr/tesseract#4569 * Modernize more code by @stweil in tesseract-ocr/tesseract#4568 * Fix compiler warnings (-Wold-style-cast) by @stweil in tesseract-ocr/tesseract#4571 * Fix some compiler warnings (-Wunused-parameter) by @stweil in tesseract-ocr/tesseract#4570 * ci: Improve installer for windows by @stweil in tesseract-ocr/tesseract#4572 * Fix several issues reported by Codacy (including real bugs) by @stweil in tesseract-ocr/tesseract#4573 * Bump actions/checkout from 6 to 7 by @dependabot[bot] in tesseract-ocr/tesseract#4574 * Fix crash when LSTM is missing in disabled-legacy build (#4448) by @gaurav0107 in tesseract-ocr/tesseract#4563 * ci: Remove unused ilammy/setup-nasm by @stweil in tesseract-ocr/tesseract#4575 * autotools: Simplify Makefile rules by @stweil in tesseract-ocr/tesseract#4579 * Fix memory-safety issues in .traineddata deserialization by @stweil in tesseract-ocr/tesseract#4581 * Handle send() failures in SVNetwork::Flush by @Ramya-9353 in tesseract-ocr/tesseract#4576 * Small code improvements by @stweil in tesseract-ocr/tesseract#4585 * doc: Add comprehensive PARAMETERS section to the tesseract man page by @stweil with @Copilot in tesseract-ocr/tesseract#4526 * Fix integer overflow in LSTM Convolve and Reconfig deserialization by @stweil in tesseract-ocr/tesseract#4588 ## New Contributors * @szepeviktor made their first contribution in tesseract-ocr/tesseract#4497 * @ppavacic made their first contribution in tesseract-ocr/tesseract#4503 * @Mohataseem89 made their first contribution in tesseract-ocr/tesseract#4509 * @markbus-ai made their first contribution in tesseract-ocr/tesseract#4511 * @eduardomozart made their first contribution in tesseract-ocr/tesseract#4516 * @gaurav0107 made their first contribution in tesseract-ocr/tesseract#4563 * @Ramya-9353 made their first contribution in tesseract-ocr/tesseract#4576 **Full Changelog**: https://github.com/tesseract-ocr/tesseract/compare/5.5.2...5.5.3</pre> <p>View the full release notes at <a href="https://github.com/tesseract-ocr/tesseract/releases/tag/5.5.3">https://github.com/tesseract-ocr/tesseract/releases/tag/5.5.3</a>.</p> </details> <hr> See merge request: Harmonybrew/homebrew-core!15091
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.
Add range and overflow validation in Convolve::DeSerialize and Reconfig::DeSerialize to prevent a crafted .traineddata file from triggering a heap out-of-bounds write via unchecked signed integer multiplication when computing the output-channel count.
Validate ni/no/num_weights in Network::CreateFromFile.
Add defense-in-depth bounds assertions in NetworkIO::Randomize and NetworkIO::CopyTimeStepGeneral.
Assisted-by: OpenCode / big-pickle (opencode)