[GH-ISSUE #312] heap-buffer-overflow in utility.c:461:12 #251

Closed
opened 2026-05-05 12:02:12 -06:00 by gitea-mirror · 7 comments
Owner

Originally created by @MrBeanc on GitHub (Nov 6, 2020).
Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/312

=================================================================
INFO: Seed: 147697476
INFO: Loaded 1 modules   (9095 inline 8-bit counters): 9095 [0x8619e0, 0x863d67), 
INFO: Loaded 1 PC tables (9095 PCs): 9095 [0x863d68,0x8875d8), 
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
==20515==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000011 at pc 0x0000005b4ba4 bp 0x7ffc1100a290 sp 0x7ffc1100a288
READ of size 1 at 0x602000000011 thread T0
    #0 0x5b4ba3 in lxw_utf8_strlen /home/mimic/vul-dig/bin-file/libxlsxwriter-master/src/utility.c:461:12
    #1 0x54a372 in worksheet_write_string /home/mimic/vul-dig/bin-file/libxlsxwriter-master/src/worksheet.c:6685:9
    #2 0x4fd3a8 in LLVMFuzzerTestOneInput /home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/fuzz.c:18:5
    #3 0x43b851 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x43b851)
    #4 0x43d88e in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x43d88e)
    #5 0x43e010 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x43e010)
    #6 0x429f7b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x429f7b)
    #7 0x455d82 in main (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x455d82)
    #8 0x7fb7854a4bf6 in __libc_start_main /build/glibc-S7xCS9/glibc-2.27/csu/../csu/libc-start.c:310
    #9 0x41e139 in _start (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x41e139)

0x602000000011 is located 0 bytes to the right of 1-byte region [0x602000000010,0x602000000011)
allocated by thread T0 here:
    #0 0x4cdbcd in malloc (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x4cdbcd)
    #1 0x7fb786705297 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x93297)
    #2 0x43d88e in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x43d88e)
    #3 0x43e010 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x43e010)
    #4 0x429f7b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x429f7b)
    #5 0x455d82 in main (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x455d82)
    #6 0x7fb7854a4bf6 in __libc_start_main /build/glibc-S7xCS9/glibc-2.27/csu/../csu/libc-start.c:310

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/mimic/vul-dig/bin-file/libxlsxwriter-master/src/utility.c:461:12 in lxw_utf8_strlen
Shadow bytes around the buggy address:
  0x0c047fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c047fff8000: fa fa[01]fa fa fa 00 02 fa fa 00 00 fa fa 00 00
  0x0c047fff8010: fa fa 00 00 fa fa 00 fa fa fa 00 fa fa fa 00 fa
  0x0c047fff8020: fa fa 00 fa fa fa 00 00 fa fa 00 00 fa fa 00 00
  0x0c047fff8030: fa fa 00 00 fa fa 00 fa fa fa 00 00 fa fa 00 00
  0x0c047fff8040: fa fa 00 00 fa fa 00 00 fa fa 00 fa fa fa 07 fa
  0x0c047fff8050: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==20515==ABORTING
MS: 0 ; base unit: 0000000000000000000000000000000000000000

The code:


#include "xlsxwriter.h"
#include<stdio.h>
int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size){

    /* Create a new workbook and add a worksheet. */
    lxw_workbook  *workbook  = workbook_new("demo.xlsx");
    lxw_worksheet *worksheet = workbook_add_worksheet(workbook, NULL);

    /* Add a format. */
    lxw_format *format = workbook_add_format(workbook);

    /* Set the bold property for the format */
    format_set_bold(format);

    /* Change the column width for clarity. */
    worksheet_set_column(worksheet, 0, 0, 20, NULL);

    worksheet_write_string(worksheet, 0, 20, data, NULL);
    workbook_close(workbook);
    printf("success!\n");
    return 0;
}

Here is my test procedure

Adding libfuzzer function when compiling libxlsxwriter

cmake . -DCMAKE_C_COMPILER=clang-9  -DCMAKE_C_FLAGS="-O2 -fno-omit-frame-pointer -g -fsanitize=address -fsanitize=fuzzer-no-link"

Then compile my test file

clang-9 fuzz.c -lxlsxwriter -fsanitize=fuzzer -fsanitize=address -lz -g

Then you can see the crash

fuzz.zip

Originally created by @MrBeanc on GitHub (Nov 6, 2020). Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/312 ``` ================================================================= INFO: Seed: 147697476 INFO: Loaded 1 modules (9095 inline 8-bit counters): 9095 [0x8619e0, 0x863d67), INFO: Loaded 1 PC tables (9095 PCs): 9095 [0x863d68,0x8875d8), INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes ==20515==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000011 at pc 0x0000005b4ba4 bp 0x7ffc1100a290 sp 0x7ffc1100a288 READ of size 1 at 0x602000000011 thread T0 #0 0x5b4ba3 in lxw_utf8_strlen /home/mimic/vul-dig/bin-file/libxlsxwriter-master/src/utility.c:461:12 #1 0x54a372 in worksheet_write_string /home/mimic/vul-dig/bin-file/libxlsxwriter-master/src/worksheet.c:6685:9 #2 0x4fd3a8 in LLVMFuzzerTestOneInput /home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/fuzz.c:18:5 #3 0x43b851 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x43b851) #4 0x43d88e in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x43d88e) #5 0x43e010 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x43e010) #6 0x429f7b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x429f7b) #7 0x455d82 in main (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x455d82) #8 0x7fb7854a4bf6 in __libc_start_main /build/glibc-S7xCS9/glibc-2.27/csu/../csu/libc-start.c:310 #9 0x41e139 in _start (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x41e139) 0x602000000011 is located 0 bytes to the right of 1-byte region [0x602000000010,0x602000000011) allocated by thread T0 here: #0 0x4cdbcd in malloc (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x4cdbcd) #1 0x7fb786705297 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x93297) #2 0x43d88e in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x43d88e) #3 0x43e010 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x43e010) #4 0x429f7b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x429f7b) #5 0x455d82 in main (/home/mimic/vul-dig/bin-file/libxlsxwriter-master/fuzz-test/a.out+0x455d82) #6 0x7fb7854a4bf6 in __libc_start_main /build/glibc-S7xCS9/glibc-2.27/csu/../csu/libc-start.c:310 SUMMARY: AddressSanitizer: heap-buffer-overflow /home/mimic/vul-dig/bin-file/libxlsxwriter-master/src/utility.c:461:12 in lxw_utf8_strlen Shadow bytes around the buggy address: 0x0c047fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0c047fff8000: fa fa[01]fa fa fa 00 02 fa fa 00 00 fa fa 00 00 0x0c047fff8010: fa fa 00 00 fa fa 00 fa fa fa 00 fa fa fa 00 fa 0x0c047fff8020: fa fa 00 fa fa fa 00 00 fa fa 00 00 fa fa 00 00 0x0c047fff8030: fa fa 00 00 fa fa 00 fa fa fa 00 00 fa fa 00 00 0x0c047fff8040: fa fa 00 00 fa fa 00 00 fa fa 00 fa fa fa 07 fa 0x0c047fff8050: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Shadow gap: cc ==20515==ABORTING MS: 0 ; base unit: 0000000000000000000000000000000000000000 ``` The code: ```C #include "xlsxwriter.h" #include<stdio.h> int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size){ /* Create a new workbook and add a worksheet. */ lxw_workbook *workbook = workbook_new("demo.xlsx"); lxw_worksheet *worksheet = workbook_add_worksheet(workbook, NULL); /* Add a format. */ lxw_format *format = workbook_add_format(workbook); /* Set the bold property for the format */ format_set_bold(format); /* Change the column width for clarity. */ worksheet_set_column(worksheet, 0, 0, 20, NULL); worksheet_write_string(worksheet, 0, 20, data, NULL); workbook_close(workbook); printf("success!\n"); return 0; } ``` ### Here is my test procedure **Adding libfuzzer function when compiling libxlsxwriter** ``` cmake . -DCMAKE_C_COMPILER=clang-9 -DCMAKE_C_FLAGS="-O2 -fno-omit-frame-pointer -g -fsanitize=address -fsanitize=fuzzer-no-link" ``` **Then compile my test file** ``` clang-9 fuzz.c -lxlsxwriter -fsanitize=fuzzer -fsanitize=address -lz -g ``` **Then you can see the crash** [fuzz.zip](https://github.com/jmcnamara/libxlsxwriter/files/5499207/fuzz.zip)
Author
Owner

@MrBeanc commented on GitHub (Nov 7, 2020):

I think it will cause the pointer to cross the boundary here

lxw_utf8_strlen(const char *str)
{
    size_t byte_count = 0;
    size_t char_count = 0;

    while (str[byte_count]) {
        if ((str[byte_count] & 0xc0) != 0x80)
            char_count++;

        byte_count++;
    }

    return char_count;
}
<!-- gh-comment-id:723450768 --> @MrBeanc commented on GitHub (Nov 7, 2020): I think it will cause the pointer to cross the boundary here ``` lxw_utf8_strlen(const char *str) { size_t byte_count = 0; size_t char_count = 0; while (str[byte_count]) { if ((str[byte_count] & 0xc0) != 0x80) char_count++; byte_count++; } return char_count; } ```
Author
Owner

@jmcnamara commented on GitHub (Mar 27, 2021):

Merged into #323

<!-- gh-comment-id:808802405 --> @jmcnamara commented on GitHub (Mar 27, 2021): Merged into #323
Author
Owner

@ANaumann85 commented on GitHub (Apr 5, 2021):

This issue is a bit delicate and consists of two problems:

  1. It seems, that you rely on the null terminator of a string. For example in the file worksheet.c is a check for the end of the string using !*string. That works only, if your string is really terminated by a 0. But if you get a zero-length array without the null-terminator, then the test does not work.

  2. The function lxw_utf8_strlen also relies on the zero-terminated string. If your string is not zero-terminated, the loop will not stop at the end of the string, but some where else in memory.

Both problems arise, if you got wrongly terminated strings. It is not clear to me, how one should handle these errors. In my point of view, these are user errors.

<!-- gh-comment-id:813285077 --> @ANaumann85 commented on GitHub (Apr 5, 2021): This issue is a bit delicate and consists of two problems: 1. It seems, that you rely on the null terminator of a string. For example in the [file worksheet.c](https://github.com/jmcnamara/libxlsxwriter/blob/9aa3d73a5bd1f18d6777f01d4652ca19940cd0e0/src/worksheet.c#L6673) is a check for the end of the string using `!*string`. That works only, if your string is really terminated by a 0. But if you get a zero-length array without the null-terminator, then the test does not work. 2. The function `lxw_utf8_strlen` also relies on the zero-terminated string. If your string is not zero-terminated, [the loop](https://github.com/jmcnamara/libxlsxwriter/blob/9aa3d73a5bd1f18d6777f01d4652ca19940cd0e0/src/utility.c#L461) will not stop at the end of the string, but some where else in memory. Both problems arise, if you got wrongly terminated strings. It is not clear to me, how one should handle these errors. In my point of view, these are user errors.
Author
Owner

@jmcnamara commented on GitHub (Apr 6, 2021):

@ANaumann85 Thanks for the analysis. I think I could add a strnlen()-like function that would be safer. I'll look into it.

<!-- gh-comment-id:814370792 --> @jmcnamara commented on GitHub (Apr 6, 2021): @ANaumann85 Thanks for the analysis. I think I could add a `strnlen()`-like function that would be safer. I'll look into it.
Author
Owner

@ANaumann85 commented on GitHub (Apr 6, 2021):

That looks like a good idea. Is there any feasible maximum string length? Does the xlsx specification give any hint on that?

At the moment the user passes only the pointer to the buffer, but without a length argument. One might add the buffer size as an additional argument.
But be aware, to be specific with the meaning of the size at this point. In my impression, most users have no clue about multi character encodings. So the buffer size in terms of bytes seems to be the only reasonable value.

<!-- gh-comment-id:814377259 --> @ANaumann85 commented on GitHub (Apr 6, 2021): That looks like a good idea. Is there any feasible maximum string length? Does the xlsx specification give any hint on that? At the moment the user passes only the pointer to the buffer, but without a length argument. One might add the buffer size as an additional argument. But be aware, to be specific with the meaning of the size at this point. In my impression, most users have no clue about multi character encodings. So the buffer size in terms of bytes seems to be the only reasonable value.
Author
Owner

@jmcnamara commented on GitHub (Apr 7, 2021):

Is there any feasible maximum string length?

Fortunately, yes. :-) All of the Excel string parameters have a size of some sort. The most comment are 32, 256 and 32K (in most cases -1B) characters. So a size check would be feasible.

I'll put together a demo branch to try it out.

<!-- gh-comment-id:814691999 --> @jmcnamara commented on GitHub (Apr 7, 2021): > Is there any feasible maximum string length? Fortunately, yes. :-) All of the Excel string parameters have a size of some sort. The most comment are 32, 256 and 32K (in most cases -1B) characters. So a size check would be feasible. I'll put together a demo branch to try it out.
Author
Owner

@jmcnamara commented on GitHub (Apr 7, 2021):

I got the example working on a Fedora VM and had a look at it in more detail.

The issues isn't that a non-null terminated string is passed to worksheet_write_string(). As far as I can see, the issue is a single byte object is created on the heap and then passed to worksheet_write_string() as a char*. Since it isn't null terminated the lxw_utf8_strlen() function reads past the end of the single byte into a region it shouldn't be reading. This is flagged by AddressSanitizer since it has instrumented boundary checks into the executable. However, I don't think this could cause a core dump in a non-instrumented code.

As such I think this is just a "garbage in garbage out" error and not exploitable code. So I'm going to close this issue (again). If anyone had a different opinion/analysis please let me know.

<!-- gh-comment-id:815316706 --> @jmcnamara commented on GitHub (Apr 7, 2021): I got the example working on a Fedora VM and had a look at it in more detail. The issues isn't that a non-null terminated string is passed to `worksheet_write_string()`. As far as I can see, the issue is a single byte object is created on the heap and then passed to `worksheet_write_string()` as a char*. Since it isn't null terminated the `lxw_utf8_strlen()` function reads past the end of the single byte into a region it shouldn't be reading. This is flagged by AddressSanitizer since it has instrumented boundary checks into the executable. However, I don't think this could cause a core dump in a non-instrumented code. As such I think this is just a "garbage in garbage out" error and not exploitable code. So I'm going to close this issue (again). If anyone had a different opinion/analysis please let me know.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/libxlsxwriter#251
No description provided.