bug: heap-buffer-overflow (read/write) in Net::load_param blob index out-of-bounds (src/net.cpp:1464)

[beilzx]

error log

find_blob_index_by_name Input failed
=================================================================
==3244058==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x511000000280 at pc 0x55e7afa62989 bp 0x7ffdc29e35d0 sp 0x7ffdc29e35c8
READ of size 8 at 0x511000000280 thread T0
    #0 0x55e7afa62988 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::_M_data() const /usr/include/c++/13/bits/basic_string.h:223:28
    #1 0x55e7afa62988 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::_M_is_local() const /usr/include/c++/13/bits/basic_string.h:264:13
    #2 0x55e7afa62988 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::operator=(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&&) /usr/include/c++/13/bits/basic_string.h:858:18
    #3 0x55e7afa62988 in ncnn::Net::load_param(ncnn::DataReader const&) /src/test/results/fuzzing_ncnn/test/ncnn/src/net.cpp:1464:46
    #4 0x55e7afa7bfee in ncnn::Net::load_param(_IO_FILE*) /src/test/results/fuzzing_ncnn/test/ncnn/src/net.cpp:2177:22
    #5 0x55e7afa2fbdb in main /src/test/fuzzing_ncnn/harness.cpp:17:9
    #6 0x7fe2811771c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #7 0x7fe28117728a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #8 0x55e7af94c194 in _start (/src/test/results/fuzzing_ncnn/test/ncnn/build/ncnn_param+0x13e194) (BuildId: 8489885d434959364ed3a3e9d6ed984798b5dc8d)

0x511000000280 is located 0 bytes after 256-byte region [0x511000000180,0x511000000280)
allocated by thread T0 here:
    #0 0x55e7afa2d641 in operator new(unsigned long) (/src/test/results/fuzzing_ncnn/test/ncnn/build/ncnn_param+0x21f641) (BuildId: 8489885d434959364ed3a3e9d6ed984798b5dc8d)
    #1 0x55e7afa811c1 in std::__new_allocator<ncnn::Blob>::allocate(unsigned long, void const*) /usr/include/c++/13/bits/new_allocator.h:151:48
    #2 0x55e7afa811c1 in std::allocator_traits<std::allocator<ncnn::Blob>>::allocate(std::allocator<ncnn::Blob>&, unsigned long) /usr/include/c++/13/bits/alloc_traits.h:482:28
    #3 0x55e7afa811c1 in std::_Vector_base<ncnn::Blob, std::allocator<ncnn::Blob>>::_M_allocate(unsigned long) /usr/include/c++/13/bits/stl_vector.h:381:33
    #4 0x55e7afa811c1 in std::vector<ncnn::Blob, std::allocator<ncnn::Blob>>::_M_default_append(unsigned long) /usr/include/c++/13/bits/vector.tcc:663:45
    #5 0x55e7afa522aa in std::vector<ncnn::Blob, std::allocator<ncnn::Blob>>::resize(unsigned long) /usr/include/c++/13/bits/stl_vector.h:1016:21
    #6 0x55e7afa522aa in ncnn::Net::load_param(ncnn::DataReader const&) /src/test/results/fuzzing_ncnn/test/ncnn/src/net.cpp:1334:20
    #7 0x55e7afa7bfee in ncnn::Net::load_param(_IO_FILE*) /src/test/results/fuzzing_ncnn/test/ncnn/src/net.cpp:2177:22
    #8 0x55e7afa2fbdb in main /src/test/fuzzing_ncnn/harness.cpp:17:9
    #9 0x7fe2811771c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #10 0x7fe28117728a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #11 0x55e7af94c194 in _start (/src/test/results/fuzzing_ncnn/test/ncnn/build/ncnn_param+0x13e194) (BuildId: 8489885d434959364ed3a3e9d6ed984798b5dc8d)

SUMMARY: AddressSanitizer: heap-buffer-overflow /src/test/results/fuzzing_ncnn/test/ncnn/src/net.cpp:1464:46 in ncnn::Net::load_param(ncnn::DataReader const&)
Shadow bytes around the buggy address:
  0x511000000000: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x511000000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x511000000100: 00 00 00 00 00 00 fa fa fa fa fa fa fa fa fa fa
  0x511000000180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x511000000200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x511000000280:[fa]fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x511000000300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x511000000380: 00 00 00 00 fa fa fa fa fa fa fa fa fa fa fa fa
  0x511000000400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x511000000480: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x511000000500: 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
==3244058==ABORTING

context

• ncnn commit: c189d88 (master)
• OS: Ubuntu 22.04
• Compiler: Clang/LLVM with AddressSanitizer (-fsanitize=address)
• Build type: Debug

how to reproduce

git clone https://github.com/Tencent/ncnn.git
cd ncnn
mkdir build && cd build

cmake .. -DCMAKE_BUILD_TYPE=Release \
         -DCMAKE_C_FLAGS="-fsanitize=address -fno-omit-frame-pointer -g" \
         -DCMAKE_CXX_FLAGS="-fsanitize=address -fno-omit-frame-pointer -g" \
         -DNCNN_BUILD_TOOLS=OFF \
         -DNCNN_BUILD_EXAMPLES=OFF

make -j$(nproc)

# Build the harness test program (calls ncnn library's load_param entry)
cat > harness.cpp << 'EOF'
#include <cstdio>
#include "net.h"
int main(int argc, char** argv) {
    if (argc < 2) return 0;
    ncnn::Net net;
    net.opt.use_vulkan_compute = false;
    FILE* fp = fopen(argv[1], "rb");
    if (!fp) return 0;
    net.load_param(fp);
    fclose(fp);
    return 0;
}
EOF

g++ -fsanitize=address -I../src -Isrc harness.cpp src/libncnn.a -fopenmp -o ncnn_param

# Run PoC reproduction
./ncnn_param poc6

Observe heap-buffer-overflow (READ of size 8 after a 256-byte heap region).

Root Cause Analysis

In src/net.cpp inside Net::load_param, the text parser pre-allocates the blob vector based on the blob_count declared in the file header:

int layer_count = 0;
int blob_count = 0;
SCAN_VALUE("%d", layer_count)
SCAN_VALUE("%d", blob_count)
// ...
d->blobs.resize((size_t)blob_count);   // pre-allocate based on header

Later, for each layer, new blobs are created on demand by indexing into d->blobs with an ever-incrementing blob_index:

Blob& blob = d->blobs[blob_index];   // <-- out-of-bounds when blob_index >= blob_count
blob.name = std::string(blob_name);

A malformed .param file can declare a small blob_count (e.g. 2) but contain layer definitions that require more blobs. When blob_index exceeds the pre-allocated size, std::vector::operator[] yields an out-of-bounds reference. The subsequent std::string move-assignment to blob.name then reads/writes memory outside the Blob vector's heap allocation, causing the heap-buffer-overflow.

The same issue exists for bottom-blob creation (the find_blob_index_by_name path).

more

This vulnerability was discovered via AFL++ fuzzing on the ncnn_param text parser entry point (load_param).

poc

poc6.zip

[dosubot]

Good find, @beilzx. Your root cause analysis is accurate. Looking at the current code, there are two unguarded blob_index uses in load_param (the text parser):

1. Bottom blob path (line 1439): Blob& blob = d->blobs[blob_index] — accessed when find_blob_index_by_name returns -1, with no check that blob_index < blob_count [1]
2. Top blob path (line 1459): Blob& blob = d->blobs[blob_index] — also unconditionally indexed before any bounds check [2]

Both paths increment blob_index without verifying it stays within the pre-allocated d->blobs vector, so a malformed .param declaring a small blob_count but many layer connections will walk past the allocation boundary.

This is the text-format (load_param) counterpart to the same class of bugs found in the binary format — there's an open PR (#6902) that adds bounds checks to load_param_bin for the binary path, but it doesn't cover the text parser [3].

The fix for the text parser should mirror what #6902 does for the binary format — add bounds checks before each d->blobs[blob_index] access:

// Before bottom blob path (line ~1439):
if (blob_index >= blob_count)
{
    NCNN_LOGE("invalid bottom blob index %d >= blob_count %d", blob_index, blob_count);
    delete layer;
    clear();
    return -1;
}

// Before top blob path (line ~1459):
if (blob_index >= blob_count)
{
    NCNN_LOGE("invalid top blob index %d >= blob_count %d", blob_index, blob_count);
    delete layer;
    clear();
    return -1;
}

Also worth noting: bottom_count and top_count are read from the file as raw integers and passed directly to layer->bottoms.resize(bottom_count) / layer->tops.resize(top_count) (lines 1430, 1456) without a sign check — a negative value would cause std::length_error. The same guard from #6902 applies here too [4].

To reply, just mention @dosu.

---

Docs are dead. Just use Dosu.

[beilzx]

I've submitted PR #6916 to fix this. Verified with AddressSanitizer — the crash no longer reproduces after the fix.