Small oob read in `qs_scanvalue` when the key is not found

[hgarrereyn]

Hi, there is a potential bug in qs_scanvalue when the key is not found.

This bug was reproduced on b3ed7b1.

Severity: unknown

unknown

Description

What crashes: crow::qs_scanvalue(key, qs, val, val_len) scans a URL/query string for a key, using a loop:

    while (qs[0] != '#' && qs[0] != '\0') {
        if (qs_strncmp(key, qs, key_len) == 0) break;
        qs += strcspn(qs, "&") + 1; // unconditionally skips one char past the match
    }

If the key is not found and there is no '&' delimiter in the remainder of the query (a perfectly valid case for a bare string or last pair), strcspn(qs, "&") returns the number of bytes up to the terminating NUL. The code then adds +1 regardless of whether an '&' was found, advancing the pointer one past the NUL terminator. The subsequent while condition dereferences qs[0], causing a one-byte heap out-of-bounds read. Under ASan this is reported as a heap-buffer-overflow(read) at query_string.h:256.

Suggested fix:

• Only advance past the delimiter if the delimiter is actually '&'. Example:

    size_t step = strcspn(qs, "&");
    qs += step;
    if (*qs == '&') qs++; else break; // hit NUL, stop

• Or break the loop when step == 0 and *qs == '\0'. Similar guard is needed anywhere this pattern is used.

POC

The following testcase demonstrates the bug:

testcase.cpp

#include <string>
#include "/fuzz/install/include/crow/query_string.h"

int main() {
    // Key is not present; query string has no '&' delimiter
    const char* key = "nonexistent";
    std::string qs(600, 'x'); // produces a long C-string via c_str()
    char out[8];
    // Triggers 1-byte heap OOB read at qs_scanvalue while advancing past NUL
    crow::qs_scanvalue(key, qs.c_str(), out, sizeof(out));
    return 0;
}

stdout

stderr

=================================================================
==1==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x5160000002d9 at pc 0x55edba5598d9 bp 0x7ffd7fffa350 sp 0x7ffd7fffa348
READ of size 1 at 0x5160000002d9 thread T0
    #0 0x55edba5598d8 in crow::qs_scanvalue(char const*, char const*, char*, unsigned long) /fuzz/install/include/crow/query_string.h:256:11
    #1 0x55edba559641 in main /fuzz/testcase.cpp:10:5
    #2 0x7f0417cf8d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #3 0x7f0417cf8e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #4 0x55edba47e3d4 in _start (/fuzz/test+0x2c3d4) (BuildId: fd8950d971d72fb1ea34f9ff7df68b0b7f5df42a)

0x5160000002d9 is located 0 bytes after 601-byte region [0x516000000080,0x5160000002d9)
allocated by thread T0 here:
    #0 0x55edba5571ed in operator new(unsigned long) (/fuzz/test+0x1051ed) (BuildId: fd8950d971d72fb1ea34f9ff7df68b0b7f5df42a)
    #1 0x7f041815ed0b in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::_M_construct(unsigned long, char) (/lib/x86_64-linux-gnu/libstdc++.so.6+0x14bd0b) (BuildId: e72c155b714bc42a767ec9c0dd94589110e5b42f)

SUMMARY: AddressSanitizer: heap-buffer-overflow /fuzz/install/include/crow/query_string.h:256:11 in crow::qs_scanvalue(char const*, char const*, char*, unsigned long)
Shadow bytes around the buggy address:
  0x516000000000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x516000000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x516000000100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x516000000180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x516000000200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x516000000280: 00 00 00 00 00 00 00 00 00 00 00[01]fa fa fa fa
  0x516000000300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x516000000380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x516000000400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x516000000480: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x516000000500: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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/CrowCpp/Crow /fuzz/src && \
    cd /fuzz/src && \
    git checkout b3ed7b1742c8e7eee8979106f96f643c106ba8a5 && \
    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 dependencies and build tools
RUN apt-get update && \
    DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
        libasio-dev \
        cmake \
        ninja-build \
        make \
        pkg-config \
    && rm -rf /var/lib/apt/lists/*

# Configure and install Crow (header-only)
WORKDIR /fuzz/src

RUN cmake -S . -B /fuzz/build \
    -G Ninja \
    -DCMAKE_C_COMPILER=clang_wrapper \
    -DCMAKE_CXX_COMPILER=clang_wrapper++ \
    -DCMAKE_INSTALL_PREFIX=/fuzz/install \
    -DCROW_BUILD_EXAMPLES=OFF \
    -DCROW_BUILD_TESTS=OFF \
    -DCROW_INSTALL=ON

RUN cmake --build /fuzz/build --target install --config Release

Build Command

clang++-17 -fsanitize=address -g -O0 -o /fuzz/test /fuzz/testcase.cpp -I/fuzz/install/include -pthread && /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 -pthread && /fuzz/test"

---

Additional Info

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

[gittiver]

can you have a look on the fix in linked PR?

[hgarrereyn]

lgtm!