Repository navigation
Fix oss-fuzz leaks from GH #209 - #292
Merged
Merged
Conversation
Unit Test Coverage ReportCoverage Regression Summary
Find details on the base coverage at https://iupac-inchi.github.io/InChI/coverage/index.html Find details on this PR's coverage by downloading coverage-reports-41686194734 and opening html/index.html |
djb-rwth
approved these changes
Nov 2, 2026
djb-rwth
left a comment
djb-rwth
left a comment
Collaborator
There was a problem hiding this comment.
Hi @cm-beilstein,
Looks fine.
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the memory-leak fixes and regression-test additions.
No open issues 🎉
What changed in this PR
Addresses the two memory leaks reported in #209 and adds regression tests for those leaks and the previously fixed isotope-handling error.
Changes:
- Frees temporary polymer bond lists after copying their contents.
- Frees the unused atom-list allocation when a neighbor list is empty.
- Adds regression tests with leak checks enabled under AddressSanitizer; detecting the isotope regression also requires an instrumented library.
| File | Description |
|---|---|
INCHI-1-TEST/tests/test_unit/test_github_209.cpp |
Adds polymer, empty-neighbor-list, and negative-isotope regression tests. |
INCHI-1-SRC/INCHI_BASE/src/ichisort.c |
Releases the unowned allocation for empty neighbor lists. |
INCHI-1-SRC/INCHI_API/libinchi/src/inchi_dll.c |
Frees temporary polymer-unit bond lists. |
Review effort: Balanced
oss-fuzz #489561592 reports a 16-byte leak allocated in SetInChIExtInputByExtOrigAtData when reading a polymer InChI. The test feeds such an InChI to GetStructFromINCHI and asks LeakSanitizer for leaks; it only detects them in ASAN builds. Refs #209 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SetInChIExtInputByExtOrigAtData frees each temporary polymer unit and its atom list, but not its bond list, leaking it (oss-fuzz #489561592). Refs #209 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
oss-fuzz #490920729 reports a 2-byte leak allocated in CreateNeighList. With no atoms and no t-groups the atom list holds only the count slot and is never stored in pp[0], so FreeNeighList cannot release it. The test creates and frees such a list; only ASAN builds detect the leak. Refs #209 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With zero atoms and t-groups no entry of pp points at pAtList, so FreeNeighList, which frees pp[0], leaks it (oss-fuzz #490920729). Free it right away in that case. Refs #209 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
oss-fuzz #488729177 reports an out-of-bounds read of nNumRemovedProtonsIsotopic in mark_alt_bonds_and_taut_groups: a lone proton with a negative isotope shift gave a negative index. d20e337 already rejects negative shifts, but its test input never reaches that code. These inputs do; with the guard removed ASAN reports a stack buffer overflow (i1-100) and a heap use after free (i1-1). Fixes #209 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
JanCBrammer
force-pushed
the
fix/209-oss-fuzz
branch
from
November 3, 2026 17:38
b186757 to
0790c20
Compare
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.
Fixes #209.
SetInChIExtInputByExtOrigAtDatainchi_dll.c)CreateNeighListichisort.c)mark_alt_bonds_and_taut_groupsThe
MakeIsoAtomStringframe in the report is the same out-of-bounds read, not a separate bug.Tests (
test_github_209) catch these only when the library is built with-fsanitize=address; CI does not do that. The leak tests call__lsan_do_recoverable_leak_check()and are compiled in only under ASAN.Not fixed (out of scope): some error paths of
SetInChIExtInputByExtOrigAtDatastill leak.