Missing bounds check in netCDF4-C++ NcVar::getDim()

[WardF]

Originally reported by @citibeth via esupport:

---

I believe that NcVar::getDim() is not bounds checking its input parameter. See below.

Thanks, -- Elizabeth

I'm causing malloc() problems with the following code:

for (int k=0; k<RANK; ++k) {
printf("    DD3.1 %ld %d\n", ncvar.getDimCount(), k);
                            ...
netCDF::NcDim dim(ncvar.getDim(k));

               }

This creates the output:

DD3.1 2 0
DD3.2
DD3.3
DD3.4
DD3.1 2 1
DD3.2
DD3.3
DD3.4
DD3.1 2 2

test_grid(86063,0x7fff729ac310) malloc: *\ error for object 0x7fd38053bdf8: pointer being freed was not allocated

*\ set a breakpoint in malloc_error_break to debug

[WardF]

The issue isn't in getDim(), insofar as it goes. It is properly bound checked, and NULL is returned when appropriate. Investigating to see where the 'pointer being freed' is coming from, if it's the test case from which the fragment was provided, or if it's internal to netcdf-cxx4.

[citibeth]

I'm pretty sure that the pointer having trouble being freed is from the fragment, not internal to netcdf-cxx4. But... when I removed the out-of-bounds call to getDim(), my problems ended.

Might the problem have to do with the subsequent NcDim constructor in:

netCDF::NcDim dim(ncvar.getDim(k));

What happens if NcDim::NcDim() is passed a null return from NcVar::getDim()?

On Tue, Jan 19, 2016 at 4:56 PM, Ward Fisher notifications@github.com wrote:

The issue isn't in getDim(), insofar as it goes. It is properly bound checked, and NULL is returned when appropriate. Investigating to see where the 'pointer being freed' is coming from, if it's the test case from which the fragment was provided, or if it's internal to netcdf-cxx4.

— Reply to this email directly or view it on GitHub https://github.com/Unidata/netcdf-cxx4/issues/26#issuecomment-173000736.

[WardF]

I'm working on a test now to validate this, but from my read of the code, dim would also be NULL. This is in line with the C library, I believe, which would be passed a destination pointer. In a similar test, the C function would return a non-fatal error code and the destination pointer would be NULL.

The documentation for getDim(), which I need to make more prominent (along with the other C++ interface documentation), says:

Gets the named NcDim object.

Parameters name Name of dimension. location Enumeration type controlling the groups to search.

Returns An NcDim object. If there are multiple objects indentied with the same name, the object closest to the current group is returned. If no valid object is found , a null node is returned.

[citibeth]

I'd be happy to do some more tests on my end before you spend more time on this. For example... I could do the out-of-bounds in situ in my example, and see if the returned dimension has dim.isNull().

On Tue, Jan 19, 2016 at 6:04 PM, Ward Fisher notifications@github.com wrote:

I'm working on a test now to validate this, but from my read of the code, dim would also be NULL. This is in line with the C library, I believe, which would be passed a destination pointer. In a similar test, the C function would return a non-fatal error code and the destination pointer would be NULL.

The documentation for getDim(), which I need to make more prominent (along with the other C++ interface documentation), says:

Gets the named NcDim object.

Parameters name Name of dimension. location Enumeration type controlling the groups to search.

Returns An NcDim object. If there are multiple objects indentied with the same name, the object closest to the current group is returned. If no valid object is found , a null node is returned.

— Reply to this email directly or view it on GitHub https://github.com/Unidata/netcdf-cxx4/issues/26#issuecomment-173016751.

[WardF]

That would be very helpful; if dim.isNull() is not true, that would definitely indicate an error.

[Sjurk]

On Windows, NcGroup::getDims (and thus getDim) fails if the file does not have any dims at all. It is the ncCheck call at line 908 in NcGroup.cpp that fails because it refers to &dimids[0] when dimids is empty. In Visual Studio 2013, debug mode, I don't catch any exception.

Edit: This refers to the latest official release from 2011, netcdf-cxx4-4.2.