bug: heap-buffer-overflow (write) in Mat::create via ParamDict::load_param (src/mat.cpp:337)

[beilzx]

error log

find_blob_index_by_name 56 failed
find_blob_index_by_name 57 failed
find_blob_index_by_name 58 failed
find_blob_index_by_name 59 failed
find_blob_index_by_name 60 failed
find_blob_index_by_name 61 failed
find_blob_index_by_name fiat failed
find_blob_index_by_name fire3/s,3,74,74,641 failed
find_blob_index_by_name 66 failed
find_blob_index_by_name 8=2 failed
find_blob_index_by_name 9=1 failed
find_blob_index_by_name Co failed
find_blob_index_by_name co failed
find_blob_index_by_name 3/relu_expand1x1 failed
find_blob_index_by_name firelu_expand3x3 failed
find_blob_index_by_name fire3/concat failed
find_blob_index_by_name -0=4,3,74,74,128 failed
find_blob_index_by_name Pooling failed
find_blob_index_by_name ol failed
find_blob_index_by_name pool3 failed
find_blob_index_by_name -23330=4,3,37,37,128 failed
find_blob_index_by_name Convolution failed
find_blob_index_by_name fire4/squeeze1x1 failed
find_blob_index_by_name fire4/squeeze1x1_fire4/relu_squeeze1x1 failed
find_blob_index_by_name -23330=4,3,37,37,32 failed
find_blob_index_by_name 0=32 failed
find_blob_index_by_name 1=4096 failed
find_blob_index_by_name 8=102 failed
find_blob_index_by_name Split failed
find_blob_index_by_name _3 failed
find_blob_index_by_name fire4/squlitncnn_0 failed
find_blob_index_by_name f1 failed
find_blob_index_by_name -2333037,32,3,37,37,32 failed
find_blob_index_by_name x1 failed
find_blob_index_by_name fire4/squeeze1nn_1 failed
find_blob_index_by_name firxpanexpand1x1_ failed
find_blob_index_by_name 9=firelu_expand3x3 failed
=================================================================
==3172928==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x509000000020 at pc 0x5602b9485c8d bp 0x7ffec93884f0 sp 0x7ffec93884e8
WRITE of size 4 at 0x509000000020 thread T0
    #0 0x5602b9485c8c in ncnn::Mat::create(int, unsigned long, ncnn::Allocator*) /src/test/results/fuzzing_ncnn/test/ncnn/src/mat.cpp:337:19
    #1 0x5602b946949b in ncnn::ParamDict::load_param(ncnn::DataReader const&) /src/test/results/fuzzing_ncnn/test/ncnn/src/paramdict.cpp:297:35
    #2 0x5602b9433302 in ncnn::Net::load_param(ncnn::DataReader const&) /src/test/results/fuzzing_ncnn/test/ncnn/src/net.cpp:1477:33
    #3 0x5602b945cfee in ncnn::Net::load_param(_IO_FILE*) /src/test/results/fuzzing_ncnn/test/ncnn/src/net.cpp:2177:22
    #4 0x5602b9410bdb in main /src/test/fuzzing_ncnn/harness.cpp:17:9
    #5 0x7fce3e8431c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #6 0x7fce3e84328a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #7 0x5602b932d194 in _start (/src/test/results/fuzzing_ncnn/test/ncnn/build/ncnn_param+0x13e194) (BuildId: 8489885d434959364ed3a3e9d6ed984798b5dc8d)

0x509000000020 is located 32 bytes before 36-byte region [0x509000000040,0x509000000064)
allocated by thread T0 here:
    #0 0x5602b93cdacb in posix_memalign (/src/test/results/fuzzing_ncnn/test/ncnn/build/ncnn_param+0x1deacb) (BuildId: 8489885d434959364ed3a3e9d6ed984798b5dc8d)
    #1 0x5602b9485b80 in fastMalloc /src/test/results/fuzzing_ncnn/test/ncnn/src/allocator.h:62:23
    #2 0x5602b9485b80 in ncnn::Mat::create(int, unsigned long, ncnn::Allocator*) /src/test/results/fuzzing_ncnn/test/ncnn/src/mat.cpp:331:30
    #3 0x5602b946949b in ncnn::ParamDict::load_param(ncnn::DataReader const&) /src/test/results/fuzzing_ncnn/test/ncnn/src/paramdict.cpp:297:35
    #4 0x5602b9433302 in ncnn::Net::load_param(ncnn::DataReader const&) /src/test/results/fuzzing_ncnn/test/ncnn/src/net.cpp:1477:33
    #5 0x5602b945cfee in ncnn::Net::load_param(_IO_FILE*) /src/test/results/fuzzing_ncnn/test/ncnn/src/net.cpp:2177:22
    #6 0x5602b9410bdb in main /src/test/fuzzing_ncnn/harness.cpp:17:9
    #7 0x7fce3e8431c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #8 0x7fce3e84328a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #9 0x5602b932d194 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/mat.cpp:337:19 in ncnn::Mat::create(int, unsigned long, ncnn::Allocator*)
Shadow bytes around the buggy address:
  0x508ffffffd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x508ffffffe00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x508ffffffe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x508fffffff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x508fffffff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x509000000000: fa fa fa fa[fa]fa fa fa 00 00 00 00 04 fa fa fa
  0x509000000080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x509000000100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x509000000180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x509000000200: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x509000000280: 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
==3172928==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 (attached poc4 is an AFL++ generated crash sample)
./ncnn_param poc4

Observe heap-buffer-overflow (WRITE of size 4 before a 36-byte heap region).

Root Cause Analysis

In src/paramdict.cpp inside ParamDict::load_param, the old-style array parser reads an array length len directly from the input stream and immediately passes it to Mat::create(len) without validating that len is non-negative:

// old style array
int len = 0;
int nscan = dr.scan("%d", &len);
if (nscan != 1)
{
    NCNN_LOGE("ParamDict read array length failed");
    return -1;
}

d->params[id].v.create(len);   // <-- no non-negative check!

When len is negative (e.g. -1), Mat::create(int _w) sets w = len. Inside Mat::create:

cstep = alignSize(w * elemsize, 16) / elemsize;
// ...
size_t totalsize = alignSize(total() * elemsize, 4);
if (totalsize > 0)
{
    data = fastMalloc(totalsize + (int)sizeof(*refcount));
}
if (data)
{
    refcount = (int*)(((unsigned char*)data) + totalsize);
    *refcount = 1;   // <-- crash here
}

Because w is int and elemsize is size_t, the expression w * elemsize promotes w to size_t. A negative w (e.g. -1) becomes SIZE_MAX after promotion, causing the intermediate cstep and total() calculations to wrap around to values near SIZE_MAX. Consequently totalsize also wraps around, making the fastMalloc argument overflow to a very small value (effectively 0), so only a tiny heap block is allocated. However, refcount = data + totalsize still uses the wrapped-around huge totalsize, which arithmetically wraps back to an address before the allocated block. The subsequent *refcount = 1 therefore writes into the heap redzone before the allocation, triggering the heap-buffer-overflow detected by AddressSanitizer.

more

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

poc

poc4.zip

[dosubot]

Your root cause analysis is accurate. In paramdict.cpp, the old-style array parser reads len from the input stream and passes it directly to Mat::create(len) with no non-negative guard [1].

In Mat::create(int _w, size_t _elemsize, ...), w is stored as-is, and then w * elemsize is computed — since elemsize is size_t, a negative w gets promoted to a huge unsigned value near SIZE_MAX [2]. This causes cstep and totalsize to wrap around to a small value, so fastMalloc allocates a tiny block, but refcount = data + totalsize uses the wrapped value to compute an address before the allocation, and *refcount = 1 writes into the heap redzone [3].

It's worth noting the same missing validation exists in load_param_bin — the binary array path also calls d->params[id].v.create(len) without checking len >= 0 [4].

The fix is to add a bounds check in both parsers immediately after reading len, before calling create:

int len = 0;
int nscan = dr.scan("%d", &len);
if (nscan != 1)
{
    NCNN_LOGE("ParamDict read array length failed");
    return -1;
}

if (len < 0)   // <-- add this guard
{
    NCNN_LOGE("ParamDict invalid array length %d", len);
    return -1;
}

d->params[id].v.create(len);

A defensive check inside Mat::create itself (e.g. early-returning when _w < 0) would provide defense-in-depth, but the primary fix belongs at the parser level where malformed input should be rejected before reaching memory allocation.

To reply, just mention @dosu.

---

Docs are dead. Just use Dosu.

[beilzx]

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