Skip to content

fix: reject negative proton isotope indexes - #256

Merged
fbaensch-beilstein merged 2 commits into
IUPAC-InChI:devfrom
carrerasdarren-cell:darren/fix-negative-isotope-oob
Aug 19, 2026
Merged

fix: reject negative proton isotope indexes#256
fbaensch-beilstein merged 2 commits into
IUPAC-InChI:devfrom
carrerasdarren-cell:darren/fix-negative-isotope-oob

Conversation

@carrerasdarren-cell

Copy link
Copy Markdown
Contributor

Summary

  • prevent a negative proton isotope delta from indexing before
    nNumRemovedProtonsIsotopic
  • add a GetStructFromINCHI() regression for public OSS-Fuzz issue
    488729177

Root cause

The single-proton cleanup path checked only that iso_atw_diff was no larger
than NUM_H_ISOTOPES. Because the field is signed, the malformed input
InChI=1/T/q+1/i12-112T produced iso_atw_diff == -112, passed that upper
bound, and accessed the three-element array at index -113.

The public testcase reproduces on current dev as an ASan two-byte stack
out-of-bounds read in mark_alt_bonds_and_taut_groups(). Requiring a positive
delta before subtracting one makes the API reject the malformed string with
its normal inchi_Ret_ERROR result.

Verification

  • exact OSS-Fuzz testcase: ASan/UBSan failure before the patch, clean rejection
    after the patch
  • focused API regression passes under ASan
  • all 17 C++ unit-test executables pass under ASan
  • CLI suite: 15 passed, 3 skipped, 10 xfailed
  • both CLI and libinchi targets build successfully
  • git diff --check passes

Type of change

  • feat - new feature (minor bump)
  • fix - bug fix (patch bump)
  • perf - performance improvement (patch bump)
  • BREAKING CHANGE - incompatible change
  • refactor / docs / chore / build / ci / test (no release)

Checklist

  • PR title follows Conventional Commits
  • Builds cleanly for both targets (./INCHI-1-TEST/build_with_cmake.sh all)
  • Compiles without new warnings on GCC/Clang/MSVC where applicable
  • Code follows .editorconfig (4-space indent, LF, no trailing whitespace)
  • Unit tests pass (ctest in the full_build test dir)
  • CLI tests pass (pytest INCHI-1-TEST/tests/test_executable)
  • New behavior is covered by tests
  • Public API / behavior changes are documented (not applicable)

Related issues

Refs #209 (GOF #488729177 only; the issue also tracks two unrelated leaks).

@fbaensch-beilstein fbaensch-beilstein left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix

@fbaensch-beilstein fbaensch-beilstein self-assigned this Aug 19, 2026
@fbaensch-beilstein
fbaensch-beilstein merged commit 7c246aa into IUPAC-InChI:dev Aug 19, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants