stcorp / coda

The Common Data Access toolset
http://stcorp.github.io/coda/doc/html/index.html
BSD 3-Clause "New" or "Revised" License
39 stars 17 forks source link

coda_recognize_file_fuzzer: ASSERT: 0 #61

Closed schwehr closed 4 years ago

schwehr commented 4 years ago
==257058==ERROR: AddressSanitizer: ABRT on unknown address 0x233d40003ec22 (pc 0x7f3e879a0602 bp 0x7ffde2b74580 sp 0x7ffde2b74458 T0)
    #6 0x55d7de156ee7 in __assert_fail base/logging.cc:106:3
    #7 0x55d7dd6be897 in coda_netcdf_basic_type_new third_party/stcorp_coda/libcoda/coda-netcdf-type.c:166:13
    #8 0x55d7dd6cf99f in read_var_array third_party/stcorp_coda/libcoda/coda-netcdf.c
    #9 0x55d7dd6cb7a8 in coda_netcdf_reopen third_party/stcorp_coda/libcoda/coda-netcdf.c:906:9
    #10 0x55d7dd6c265f in reopen_with_backend third_party/stcorp_coda/libcoda/coda-product.c:404:17
    #11 0x55d7dd6bf328 in open_file third_party/stcorp_coda/libcoda/coda-product.c:552:9
    #12 0x55d7dd6bec6a in coda_recognize_file third_party/stcorp_coda/libcoda/coda-product.c:596:9
    #13 0x55d7dd5d4af1 in LLVMFuzzerTestOneInput third_party/stcorp_coda/fuzz/coda_recognize_file_fuzzer.cc:19:3

The issue happens when nc_type is 0 in coda_netcdf_basic_type_new():


coda_netcdf_basic_type *coda_netcdf_basic_type_new(int nc_type, int64_t offset, int record_var, int64_t length)
{
    coda_netcdf_basic_type *type;
    coda_native_type read_type;
    int64_t byte_size;

    type = malloc(sizeof(coda_netcdf_basic_type));
    if (type == NULL)
    {
        coda_set_error(CODA_ERROR_OUT_OF_MEMORY, "out of memory (could not allocate %lu bytes) (%s:%u)",
                       (long)sizeof(coda_netcdf_basic_type), __FILE__, __LINE__);
        return NULL;
    }
    type->backend = coda_backend_netcdf;
    type->definition = NULL;  // <-- NOTE: definition is NULL causing trouble when cleaning up
    type->attributes = NULL;
    type->offset = offset;
    type->record_var = record_var;

    switch (nc_type)
    {
        case 1:
            read_type = coda_native_type_int8;
            byte_size = 1;
            type->definition = (coda_type *)coda_type_number_new(coda_format_netcdf, coda_integer_class);
            break;
// [SNIP]
        case 6:
            read_type = coda_native_type_double;
            byte_size = 8;
            type->definition = (coda_type *)coda_type_number_new(coda_format_netcdf, coda_real_class);
            break;
        default:
            assert(0);  // <-- Assert hit here.
            exit(1);
    }

I have a proposed fix that will follow in a PR. CODA_ERROR_INVALID_FORMAT was my best guess as to what the error number should be.

        default:
            coda_set_error(CODA_ERROR_INVALID_FORMAT,
                           "unsupported nc_type: %d (%s:%u)",
                           nc_type, __FILE__, __LINE__);
            coda_dynamic_type_delete((coda_dynamic_type *)type);
            return NULL;
    }

This then triggers trouble in coda_netcdf_type_delete():

void coda_netcdf_type_delete(coda_dynamic_type *type)
{
    assert(type != NULL);
    assert(type->backend == coda_backend_netcdf);

    if (type->definition->type_class == coda_array_class)  // <-- TROUBLE: definition is NULL
    {
        if (((coda_netcdf_array *)type)->base_type != NULL)
        {
            coda_dynamic_type_delete((coda_dynamic_type *)((coda_netcdf_array *)type)->base_type);
        }
    }

So a simple check will allow cleanup to work properly:

    if (type->definition != NULL &&
        type->definition->type_class == coda_array_class)

testcase-5695370323034112.zip

svniemeijer commented 4 years ago

Fixed in f7a9823bdeb3dfb6e5c88f742cee4adb6d6a1903

schwehr commented 4 years ago

verified