libsrc: bound nchars against the v1hs stream before allocating NC_string

[MarkLee131]

Summary

v1h_get_NC_string() reads nchars from the header via v1h_get_size_t() and immediately calls new_NC_string(nchars, NULL), which mallocs nchars + sizeof(NC_string) + 1 bytes. The proportionality check against the v1hs stream (check_v1hs(gsp, _RNDUP(nchars, X_ALIGN))) only runs after the allocation. A 193-byte CDF v2 file that declares a 4 GB attribute name therefore issues a malloc(0x100000010) before we ever notice the bytes aren't there. On default Linux the kernel OOM-kills the process; with overcommit, the read fills the buffer.

PR #3153 added a SIZE_MAX overflow guard inside new_NC_string, but that only catches raw size_t overflow -- a 4 GB nchars passes through and asks the allocator for ~4 GB.

Reproducer

193 bytes via the public API:

#include <stdio.h>
#include <netcdf.h>
#include <netcdf_mem.h>

static unsigned char poc[193] = {
    0x43,0x44,0x46,0x02, 0x00,0x2f,0x00,0x4c, 0xff,0xef,0x01,0x00, 0x00,0x00,0x00,0x00,
    0x00,0x00,0x00,0x0c, 0x0c,0x0c,0x0c,0x0c, 0xff,0xff,0xff,0xff, 0xff,0xff,0xff,0xff,
    0xff,0xff,0xff,0xff, 0xff,0xff,0xff,0xff, 0xff,0xff,0xff,0x00, 0x00,0x00,0x00,0x00,
    0xd0,0x90,0x90,0x90, 0x90,0x90,0x01,0x00, 0x01,0x00,0x6e,0x00, 0x00,0x00,0x00,0x00,
};

int main(void) {
    int ncid;
    nc_open_mem("/tmp/poc.nc", 0, sizeof(poc), poc, &ncid);
    return 0;
}

clang -fsanitize=address -g -O1 poc.c $(pkg-config --cflags --libs netcdf) -o poc
ASAN_OPTIONS=allocator_may_return_null=1:max_allocation_size_mb=2048 ./poc

ASan output (current main):

==...==WARNING: AddressSanitizer failed to allocate 0x100000010 bytes
    #7 malloc
    #8 new_NC_string                libdispatch/dstring.c:245
    #9 v1h_get_NC_string            libsrc/v1hpg.c:319
    #10 v1h_get_NC_attr             libsrc/v1hpg.c:755
    #11 v1h_get_NC_attrarray        libsrc/v1hpg.c:904
    #12 nc_get_NC                   libsrc/v1hpg.c:1555
    #13 NC3_open                    libsrc/nc3internal.c:1198
    #14 nc_open_mem                 libdispatch/dfile.c:823

malloc(0x100000010) ≈ slen 0xFFFFFFEF + sizeof(NC_string) + 1. slen is the nchars field that the malformed header declares for one of the attribute names.

Fix

Move the existing check_v1hs(gsp, _RNDUP(nchars, X_ALIGN)) call before new_NC_string. No new logic -- the check that used to run after the malloc just runs first. Catches the bomb at the source without changing any other behavior; the post-allocation check_v1hs becomes redundant and is removed.

This complements PR #3153 rather than replaces it; the SIZE_MAX overflow guard added there is still a useful last line of defense.

[edhartnett]

Firstly, thank you for your contribution to netCDF. Without community input and support, netCDF would not be what it is today.

I would like to ask a few questions:

Is this work AI generated?

Was there a bug or issue that describes this?

Is there a way to generate the problematic file with the netCDF API? It's not clear to me that we should try to solve every problem with malformed header information, unless it's the result of a bug in netCDF.

I wonder, how did you find this?

[MarkLee131]

@edhartnett AI: no.

Found via libFuzzer + ASan against an nc_open_mem harness with -malloc_limit_mb=2048.

PR #3153 hardened the same new_NC_string for the size_t-overflow case; that guard catches raw overflow but a 4 GB nchars fits in size_t on 64-bit and slips through. check_v1hs(gsp, _RNDUP(nchars, X_ALIGN)) was already in v1h_get_NC_string; this PR just moves it before the malloc instead of after.

nc_open accepts attacker-controlled bytes; a 193-byte input asking for 4 GB is a DoS amplifier regardless of whether netCDF's writer can produce it. #3153 already settled this direction.

[MarkLee131]

Can file a tracking issue first if you'd prefer.