Off-by-one overflow (write) in `bagLegacyToWkt`

[hgarrereyn]

Hi, there is a potential bug in bagLegacyToWkt reachable when the buffer is too small to hold the full result.

This bug was reproduced on f4d7860.

Description

The function bagLegacyToWkt is used to convert a bag coordinate system to wkt. It takes two optional output buffers (horizontal and vertical) each with their size. If the provided buffers are too small to hold the full resulting string, it tries to truncate the string so it fits. However, there is an off-by-one in the truncation math (the null byte is not accounted for), so the resulting output overflows by one null-byte.

Specifically, this code is incorrect:

BAG/api/bag_legacy_crs.cpp

Lines 559 to 563 in f4d7860

	//Make sure our string is not too large.
	if (wktStream.str().size() > vBufferSize)
	wktStream.str().resize(vBufferSize);
	strcpy(vBuffer, wktStream.str().c_str());

strcpy will copy the full string and also append a null byte at the end. Thus an extra byte should be accommodated when truncating to allow for this null byte.

This pattern exists in both the horizontal and vertical truncation paths. The testcase below shows just the vertical case.

POC

The following testcase demonstrates the bug:

testcase.cpp

#include <cstdint>
#include <cstdlib>
#include <cstring>
#include "/fuzz/install/include/bag_legacy_crs.h"

int main() {
    BAG::BagLegacyReferenceSystem sys{};
    sys.coordSys = BAG::CoordinateType::Geodetic;
    auto &p = sys.geoParameters;
    p.datum = BAG::BagDatum::wgs84;
    std::strcpy(p.ellipsoid, "WGS 84");
    std::strcpy(p.vertical_datum, "MSL");

    /// buffer is too small to hold the full result
    char vbuf[5];

    // Null horizontal buffer; only vertical path executed.
    (void)BAG::bagLegacyToWkt(sys, nullptr, 0, vbuf, 5);
    return 0;
}

stdout

=================================================================
==1==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f7a8a300715 at pc 0x55f2a7375f00 bp 0x7fffc431d2f0 sp 0x7fffc431caa8
WRITE of size 40 at 0x7f7a8a300715 thread T0
    #0 0x55f2a7375eff in strcpy (/fuzz/test+0xb4eff) (BuildId: 2667f3eb20206182e78443c0f931174afc9407bc)
    #1 0x55f2a73cddeb in BAG::bagLegacyToWkt(BAG::BagLegacyReferenceSystem const&, char*, unsigned long, char*, unsigned long) (/fuzz/test+0x10cdeb) (BuildId: 2667f3eb20206182e78443c0f931174afc9407bc)
    #2 0x55f2a73cd668 in main /fuzz/testcase.cpp:18:11
    #3 0x7f7a8eb58d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #4 0x7f7a8eb58e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #5 0x55f2a72f1e54 in _start (/fuzz/test+0x30e54) (BuildId: 2667f3eb20206182e78443c0f931174afc9407bc)

Address 0x7f7a8a300715 is located in stack of thread T0 at offset 789 in frame
    #0 0x55f2a73ccf5f in main /fuzz/testcase.cpp:6

  This frame has 2 object(s):
    [32, 656) 'sys' (line 7)
    [784, 789) 'vbuf' (line 15) <== Memory access at offset 789 overflows this variable
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+0xb4eff) (BuildId: 2667f3eb20206182e78443c0f931174afc9407bc) in strcpy
Shadow bytes around the buggy address:
  0x7f7a8a300480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f7a8a300500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f7a8a300580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f7a8a300600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f7a8a300680: 00 00 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2
=>0x7f7a8a300700: f2 f2[05]f3 f3 f3 f3 f3 00 00 00 00 00 00 00 00
  0x7f7a8a300780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f7a8a300800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f7a8a300880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f7a8a300900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7f7a8a300980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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

stderr

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/OpenNavigationSurface/BAG /fuzz/src && \
    cd /fuzz/src && \
    git checkout f4d7860efa36c6f1fd31a5c50d3656f6b80bb41b && \
    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++

RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
    build-essential \
    cmake \
    ninja-build \
    libhdf5-dev \
    libxml2-dev \
    pkg-config \
 && rm -rf /var/lib/apt/lists/*

WORKDIR /fuzz/src
RUN cmake -S . -B /tmp/build -G Ninja \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_C_COMPILER=clang_wrapper \
    -DCMAKE_CXX_COMPILER=clang_wrapper++ \
    -DCMAKE_INSTALL_PREFIX=/fuzz/install \
    -DBAG_BUILD_SHARED_LIBS=OFF \
    -DBAG_BUILD_TESTS=OFF \
    -DBAG_BUILD_EXAMPLES=OFF \
    -DBAG_BUILD_PYTHON=OFF
RUN cmake --build /tmp/build --target install -j$(nproc)

Build Command

clang++-17 -fsanitize=address -g -O0 -o /fuzz/test /fuzz/testcase.cpp -I/fuzz/install/include -L/fuzz/install/lib -L/fuzz/install/lib/static -L/usr/lib/x86_64-linux-gnu/hdf5/serial -lbaglib -lxml2 -lhdf5_cpp -lhdf5 -lz -ldl -lm -lpthread && /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 -L/fuzz/install/lib/static -L/usr/lib/x86_64-linux-gnu/hdf5/serial -lbaglib -lxml2 -lhdf5_cpp -lhdf5 -lz -ldl -lm -lpthread && /fuzz/test"

---

Additional Info

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