Re: use of assert() macro to check file problems

• To: Michael McCarrick <mmccarri@xxxxxxxx>
• Subject: Re: use of assert() macro to check file problems
• From: "Glenn P. Davis" <davis@xxxxxxxxxxxxxxxxxxxxxxx>
• Date: Fri, 26 Jun 1998 13:26:54 -0600

> Hello,
>
> There are several places in the netCDF library where the assert()
> macro is the only check on file problems.  For example, in
> v1h_get_nc_type(), assert checks for a valid netCDF var type.  The
> problem is, if an nc file gets corrupted (for example, truncated during
> a copy etc.) this is the only way to catch the error.  If assert()
> catches the error, the program aborts.
>
> I have a distributed server application that allows remote access to netCDF
> files through a CORBA interface.  If the server is linked with a version
> of the netCDF lib that includes assert(), then the server will crash
> (abort) as soon as a corrupted file is opened.  This, of course, is
> unacceptable for a server process.  On the other hand, if I link with a
> NDEBUG version of the lib, the file errors are ignored and garbage is
> returned.  It would be much better if these assert()'s were replaced with
> code to return an error value.  My understanding is that assert() should
> only be used to catch programmer errors, not as an error handling for
> external input (netCDF flies).
>
> Thanks,
>
> Mike McCarrick

Our intention is that the assert() macro be used as a check on
programming errors as opposed to run time errors. We recommend
that the library be compiled -NDEBUG for normal use, and our
'configure' script generally enforces this. So, I'm not receptive
to compliants that your program aborts on an assertion failure at
runtime. The point I'm trying to make is that you are accusing us of
"using assert() macro to check file problems", which is untrue.
The situation is in fact worse, this layer assumes that if the magic
number is valid, then this is a valid netcdf file as defined by the BNF.


You could perhaps argue that
v1h_get_nc_type()
could be more restrictive in what it accepts from the file.
Perhaps changing
        assert(type == NC_BYTE
                || type == NC_CHAR
                || type == NC_SHORT
                || type == NC_INT
                || type == NC_FLOAT
                || type == NC_DOUBLE);
to
        if(!(type == NC_BYTE
                        || type == NC_CHAR
                        || type == NC_SHORT
                        || type == NC_INT
                        || type == NC_FLOAT
                        || type == NC_DOUBLE)
        {
                return NC_ENOTNC; /* ?? what is the appropriate error code */
        }

The rational for checking this as an assertation is that we are checking the
consistancy of the code which wrote this netcdf. EG, In this layer, I'm not
really dealing with "corruption" such as you anticipate beyond the magic number
check.

In the particular case of nc_type, it should be the case that
any use an invalid value (after conversion to enum nc_type) will generate
runtime errors in the higher layers. If not, please send a small example file
and I'll look at it.

It is certainly possible to produce a file that looks like a netcdf file
to 'get' code of v1hpg.c but which will cause bizarre and unpredicable results.
Any of the size numbers could be whacky and there is no way to cross check
them beyond some sanity checks. And of course, how can we check that the data
hasn't been tampered with?

The answer is to use the right tools for copying and check your work
when copying. If you are really concerned about data security, then
you have to go to a level above netcdf. I would suggest carrying an MD5
checksum in a separate associated file. Generate checksums after the netcdf
is closed, verify the checksum prior to open.

-glenn

• References:
  • use of assert() macro to check file problems
    • From: Michael McCarrick