NC4_get_vars(): fix out-of-bounds write with unlimited dimension

[rouault]

This fixes an issue hit by GDAL, and that is found in netcdf 4.6.3
and 4.7.0

git bisect pointed the problem to have started with

77ab979c5fd7151f8583c7698498bb8cb0dafb63 is the first bad commit
commit 77ab979c5fd7151f8583c7698498bb8cb0dafb63
Author: Ed Hartnett <edwardjameshartnett@gmail.com>
Date:   Sat Jun 16 09:58:48 2018 -0600

    using get_vars but not put_vars

:040000 040000 8611e77aaea4f130ca769a852e2c3b26373a78fa fc9ffd1d13d8f5db5ab888044e0cec4def26eba1 M	libsrc4

where nc_get_vara_double() started using nc4_get_vars() underneath.

It turns out that nc4_get_vars() was buggy in the situation exercised by GDAL.

This can be reproduced with the following simple test case:

int main()
{
    int status;
    int cdfid = -1;
    int first_dim;
    int varid;
    int other_var;
    size_t anStart[NC_MAX_DIMS];
    size_t anCount[NC_MAX_DIMS];
    double* val = (double*)calloc(3, sizeof(double));

    status = nc_create("foo.nc", NC_NETCDF4, &cdfid);
    assert( status == NC_NOERR );

    status = nc_def_dim(cdfid, "unlimited_dim", NC_UNLIMITED, &first_dim);
    assert( status == NC_NOERR );

    status = nc_def_var(cdfid, "my_var", NC_DOUBLE, 1, &first_dim, &varid);
    assert( status == NC_NOERR );

    status = nc_def_var(cdfid, "other_var", NC_DOUBLE, 1, &first_dim, &other_var);
    assert( status == NC_NOERR );

    status = nc_enddef(cdfid);
    assert( status == NC_NOERR );

    /* Write 3 elements to set the size of the unlimited dim to 3 */
    anStart[0] = 0;
    anCount[0] = 3;
    status = nc_put_vara_double(cdfid, other_var, anStart, anCount, val);
    assert( status == NC_NOERR );

    /* Read 2 elements starting with index=1 */
    anStart[0] = 1;
    anCount[0] = 2;
    status = nc_get_vara_double(cdfid, varid, anStart, anCount, val);
    assert( status == NC_NOERR );

    status = nc_close(cdfid);
    assert( status == NC_NOERR );

    free(val);

    return 0;
}

Running it under Valgrind without this patch leads to

==19637==
==19637== Invalid write of size 8
==19637==    at 0x4C326CB: memcpy@@GLIBC_2.14 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19637==    by 0x4EDBE3D: NC4_get_vars (hdf5var.c:2131)
==19637==    by 0x4EDA24C: NC4_get_vara (hdf5var.c:1342)
==19637==    by 0x4E68878: NC_get_vara (dvarget.c:104)
==19637==    by 0x4E69FDB: nc_get_vara_double (dvarget.c:815)
==19637==    by 0x400C08: main (in /home/even/netcdf-c/build/test)
==19637==  Address 0xb70e3e8 is 8 bytes before a block of size 24 alloc'd
==19637==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19637==    by 0x4009E8: main (in /home/even/netcdf-c/build/test)
==19637==

[CLAassistant]

All committers have signed the CLA.

[edhartnett]

Could you also incorporate the test code to confirm this fix?

[rouault]

Could you also incorporate the test code to confirm this fix?

Done per 0c7be1d

[edhartnett]

Awesome but don't forget the Makefile.am. The test must be added there as well.

[edhartnett]

Thanks!

[WardF]

Thanks!