stcorp / coda

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

coda_recognize_file_fuzzer: Crash in read_bytes #53

Closed schwehr closed 4 years ago

schwehr commented 4 years ago
==614332==ERROR: AddressSanitizer: SEGV on unknown address 0x7f5369c00fff (pc 0x7f536ce755a0 bp 0x7ffdca355690 sp 0x7ffdca354e48 T0)
--
  | ==614332==The signal is caused by a READ memory access.
  | #0 0x7f536ce755a0 in __memmove_ssse3_back (/usr/grte/v4/lib64/libc.so.6+0xc35a0)
  | #1 0x5597d854d4cc in read_bytes third_party/stcorp_coda/libcoda/coda-read-bytes.h:79:9
  | #2 0x5597d854e2d2 in read_GDR third_party/stcorp_coda/libcoda/coda-cdf.c:1107:9
  | #3 0x5597d854de33 in read_file third_party/stcorp_coda/libcoda/coda-cdf.c:1297:9
  | #4 0x5597d854d044 in coda_cdf_reopen third_party/stcorp_coda/libcoda/coda-cdf.c:1380:9
  | #5 0x5597d854ca9a in reopen_with_backend third_party/stcorp_coda/libcoda/coda-product.c:396:17
  | #6 0x5597d85497a8 in open_file third_party/stcorp_coda/libcoda/coda-product.c:550:9
  | #7 0x5597d85490ea in coda_recognize_file third_party/stcorp_coda/libcoda/coda-product.c:594:9
  | #8 0x5597d845f8f1 in LLVMFuzzerTestOneInput third_party/stcorp_coda/fuzz/coda_recognize_file_fuzzer.cc:19:3

testcase-5685773629652992.zip

schwehr commented 4 years ago

gdr_offset is -9 when calling read_GDR. This is probably not quite the right solution. My guess is that the offset arg to read_GDR should actually always be >= 0, yes? at coda-cdf.c:1107:

static int read_GDR(coda_cdf_product *product_file, int64_t offset)
{
    int32_t record_type;
    int64_t rvdr_head;
    int64_t zvdr_head;
    int64_t adr_head;
    int64_t eof;
    int32_t nr_vars;
    int32_t num_attr;
    int32_t nz_vars;

    if (offset < -8)
    {
        coda_set_error(CODA_ERROR_PRODUCT, "CDF file has invalid offset (%ld) for GDR record", offset);
        return -1;
    }
svniemeijer commented 4 years ago

I added checks on (I hope) all the variables that could result in a negative offset: c0e662b2c110d40a79007105aaa1344d4377bdc5

Let me know if you find anything else.

schwehr commented 4 years ago

This looks to have caught 3 additional issues that the fuzzer had. I have just over 10 more that need to add for this fuzzer. I will add them as I get time. Alternatively, you can sign up for ossfuzz. And you are welcome to any of the fuzzers I've made. They are all pretty simple and I am sure you could improve them quite a bit.

svniemeijer commented 4 years ago

I can't seem to find any information on how to sign up. Do you have any pointers?

schwehr commented 4 years ago

https://google.github.io/oss-fuzz/getting-started/new-project-guide/ has the instructions. When you get to the bottom, it says to send a pull requests to add the project

svniemeijer commented 4 years ago

It seems to still take quite some steps after this pull request. And if I understand it correctly then we would have to add all the fuzz tests to the CODA repository (or some other repo of our own)? We are not looking to maintain a fuzz repo for our repositories. However, we are still willing to look into issues that are found by whoever finds issues via fuzzing.

svniemeijer commented 4 years ago

Also, I don't see HDF5 in the OSS-Fuzz build status. Is this library also fuzz tested? We definitely won't be able to take on responsibility for fuzz problems in our external dependencies.

schwehr commented 4 years ago

You don't have to take on the responsibility of issues for the deps. GDAL already fuzzes into hdf5, but probably doesn't get very far in it. I fuzz hdf5 a bit deeper in gdal than the external fuzzers and haven't hit much myself. If you hit a bug in hdf5, just let the hdf folks know and let it go.

svniemeijer commented 4 years ago

I added a much more generic check against negative offsets in bb512d343bd6397803ec4b5a135106b54c492c46. This might eliminate a few more cases.