brechtsanders / xlsxio

XLSX I/O - C library for reading and writing .xlsx files
MIT License
425 stars 113 forks source link

Crashes opening this file #27

Closed webern closed 6 years ago

webern commented 6 years ago

For me, xlsxio crashes on the attached file. I have a highly customized build, though, so I would be interested to know if you can open this file with xlsxio. (My build is wonky because I'm using xlsxio in a Node.js addon, here.)

* thread #10, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x00007fff65e1a232 libsystem_c.dylib`strlen + 18
    frame #1: 0x000000010aadf086 xlsx.node`unzLocateFile(file=0x0000000104c13140, szFileName=0x0000000000000000, iCaseSensitivity=0) at unzip.c:1253
    frame #2: 0x000000010aaa6fef xlsx.node`XML_Char_openzip(archive=0x0000000104c13140, filename=0x0000000000000000, flags=0) at xlsxio_read.c:43
    frame #3: 0x000000010aaa6e52 xlsx.node`expat_process_zip_file(zip=0x0000000104c13140, filename=0x0000000000000000, start_handler=(xlsx.node`shared_strings_callback_find_sharedstringtable_start at xlsxio_read_sharedstrings.c:125), end_handler=0x0000000000000000, data_handler=0x0000000000000000, callbackdata=0x0000700007a30ec0, xmlparser=0x0000700007a30ec0) at xlsxio_read.c:139
    frame #4: 0x000000010aaa9ade xlsx.node`xlsxioread_process(handle=0x0000000104c058b0, sheetname=0x0000000000000000, flags=129, cell_callback=0x0000000000000000, row_callback=0x0000000000000000, callbackdata=0x0000000104c1cb50) at xlsxio_read.c:1223
  * frame #5: 0x000000010aaaa05f xlsx.node`xlsxioread_sheet_open(handle=0x0000000104c058b0, sheetname=0x0000000000000000, flags=1) at xlsxio_read.c:1350
    frame #6: 0x000000010aa75ff8 xlsx.node`xlsx::extractAllRows(sheetname=0x0000000000000000, xreader=0x0000700007a349e8, hasHeaders=false, headerTransformMap=size=0, deletes=size=0, doPascalCase=false, pascalWords=size=0) at XlsxReaderFunctions.h:73
    frame #7: 0x000000010aa73674 xlsx.node`xlsx::extractAllData(filename="/Users/mjb/Documents/repos/xlsx-util/tests/sci-bug.xlsx", hasHeaders=false, headerTransformMap=size=0, deletes=size=0, doPascalCase=false, pascalWords=size=0) at XlsxReaderFunctions.h:52
    frame #8: 0x000000010aa72b7c xlsx.node`xlsx::AsyncReader::Execute(this=0x00000001061139f0) at AsyncReader.cpp:34
    frame #9: 0x000000010aa700c9 xlsx.node`Napi::AsyncWorker::OnExecute(env=0x0000000104d2bbb0, this_pointer=0x00000001061139f0) at napi-inl.h:3170
    frame #10: 0x00000001000937ff node`(anonymous namespace)::uvimpl::Work::ExecuteCallback(req=0x000000010610dd60) at node_api.cc:3363
    frame #11: 0x000000010171c201 node`uv__queue_work(w=0x000000010610ddb8) at threadpool.c:259
    frame #12: 0x000000010171c8e6 node`worker(arg=0x0000000000000000) at threadpool.c:83
    frame #13: 0x0000000104b65665 libsystem_pthread.dylib`_pthread_body + 340
    frame #14: 0x0000000104b65511 libsystem_pthread.dylib`_pthread_start + 377
    frame #15: 0x0000000104b64bfd libsystem_pthread.dylib`thread_start + 13

image

sci-bug.xlsx

brechtsanders commented 6 years ago

Just tried it on my side and it crashes too (see screenshot below). The crash happens inside MiniZip function unzLocateFile, same as with you.

screenshot

brechtsanders commented 6 years ago

Found it! Apparently sometimes XML_Char_openzip gets called with filename set to NULL. I fixed it so now that function just returns. Please try again with version 0.2.19? For me it works now with your file.

webern commented 6 years ago

I didn't see this and I think I also found it. I was seeing that, when there is no sharedstrings file, then there is a crash on strlen(NULL), so the fix that I found was:

xlsxio_read.c (1223)

  if ((getrelscallbackdata.sharedstringsfile == NULL) || expat_process_zip_file(handle->zip, getrelscallbackdata.sharedstringsfile, shared_strings_callback_find_sharedstringtable_start, NULL, NULL, &sharedstringsdata, &sharedstringsdata.xmlparser) != 0) {
    //no shared strings found
    sharedstringlist_destroy(sharedstrings);
    sharedstrings = NULL;
  }
brechtsanders commented 6 years ago

You are also right. My fix catched this case where getrelscallbackdata.sharedstringsfile is NULL further down the line. I will add an additional check to that part of the code too just to be on the safe side.