[GH-ISSUE #486] NULL Pointer Dereference in libxlsxwriter Chart Processing #380

New issue

Closed

opened 2026-05-05 12:13:49 -06:00 by gitea-mirror · 2 comments

gitea-mirror commented

2026-05-05 12:13:49 -06:00

Owner

Originally created by @LkkkLxy on GitHub (Sep 29, 2025).
Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/486

Originally assigned to: @jmcnamara on GitHub.

Describe the bug

A NULL pointer dereference vulnerability exists in libxlsxwriter's chart processing workflow. The issue stems from inadequate input validation in workbook_add_chart() which accepts invalid chart types, leading to incomplete initialization of chart structures. When workbook_close() attempts to process these malformed charts, a NULL pointer dereference occurs in _write_chart_files().

Root Cause Analysis

Technical Details

Input Validation Failure: workbook_add_chart(workbook, 0) accepts invalid chart type 0
Incomplete Initialization: _chart_initialize() only prints a warning for invalid types but doesn't initialize critical function pointers
NULL Pointer Dereference: _write_chart_files() calls lxw_chart_assemble_xml_file() which attempts to call NULL function pointers
Crash Location: SEGV occurs at address 0x000000000000 in packager.c:540

Call Chain

workbook_close() 
  → lxw_create_package()
    → _write_chart_files() 
      → lxw_chart_assemble_xml_file()
        → (*chart->write_chart_type)() // NULL function pointer call
          → SEGV at 0x000000000000

To Reproduce

Ultra-minimal reproduction code (6 APIs, tested and verified):

#include <xlsxwriter.h>
#include <stdio.h>

int main() {
    printf("ULTRA MINIMAL: 6 APIs only - libxlsxwriter crash\n");
    
    // 1. Create workbook
    lxw_workbook *workbook = workbook_new("test.xlsx");
    if (!workbook) return 1;
    
    // 2. Add worksheet (required for chart insertion)
    lxw_worksheet *worksheet = workbook_add_worksheet(workbook, NULL);
    if (!worksheet) {
        workbook_close(workbook);
        return 1;
    }
    
    // 3. Create chart with INVALID type 0 - KEY TRIGGER
    lxw_chart *chart = workbook_add_chart(workbook, 0);
    if (chart) {
        // 4. Add series (required - chart must have series)
        lxw_chart_series *series = chart_add_series(chart, "", "");
        
        // 5. Insert chart into worksheet (required to trigger processing)
        worksheet_insert_chart(worksheet, 0, 0, chart);
    }
    
    printf("Calling workbook_close - crash expected...\n");
    
    // 6. Close workbook - TRIGGERS NULL POINTER DEREFERENCE
    workbook_close(workbook);
    
    printf("ERROR: Should never reach this line!\n");
    return 0;
}

Compilation and Testing

# In libxlsxwriter docker container
gcc -g -fsanitize=address -I/src/libxlsxwriter/include \
    ultra_minimal.c -L/src/libxlsxwriter-install/lib -lxlsxwriter -lz \
    -o ultra_minimal && ./ultra_minimal

Expected behavior

The library should:

Validate chart types during creation and reject invalid types with clear error codes
Fail gracefully for invalid inputs rather than creating partially initialized objects
Never crash regardless of input combination

static lxw_error _write_chart_files(lxw_packager *self) {
    // ...
    lxw_chart_assemble_xml_file(chart);  // Calls NULL function pointer
}

Version

libxlsxwriter version: Current git master
OS and version: Linux x86_64
Compiler: clang with AddressSanitizer
Build configuration: Docker container build
Testing: Verified reproduction in libxlsxwriter docker environment

Additional Context

This bug was discovered through fuzz testing.

By examining the code, I believe that since LXW_CHART_NONE is defined in the enum, it is inappropriate to leave it unhandled. This does not fall under API misuse. It can be addressed simply with an if check, or even with an assert. (Of course, for values outside the range of the enum provided to users, we don’t need to handle them — that would be API misuse.)

Of course, there are other higher-cost approaches, but I don’t think it is appropriate to introduce significant overhead for such a minor bug. Since I am a fuzz tester and lack prior knowledge of this library, my proposed solution may not be entirely accurate. If you have other ideas, I’d be happy to discuss them.

Originally created by @LkkkLxy on GitHub (Sep 29, 2025). Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/486 Originally assigned to: @jmcnamara on GitHub. ## Describe the bug A NULL pointer dereference vulnerability exists in libxlsxwriter's chart processing workflow. The issue stems from inadequate input validation in `workbook_add_chart()` which accepts invalid chart types, leading to incomplete initialization of chart structures. When `workbook_close()` attempts to process these malformed charts, a NULL pointer dereference occurs in `_write_chart_files()`. ## Root Cause Analysis ### Technical Details 1. **Input Validation Failure**: `workbook_add_chart(workbook, 0)` accepts invalid chart type `0` 2. **Incomplete Initialization**: `_chart_initialize()` only prints a warning for invalid types but doesn't initialize critical function pointers 3. **NULL Pointer Dereference**: `_write_chart_files()` calls `lxw_chart_assemble_xml_file()` which attempts to call NULL function pointers 4. **Crash Location**: SEGV occurs at address 0x000000000000 in packager.c:540 ### Call Chain ``` workbook_close() → lxw_create_package() → _write_chart_files() → lxw_chart_assemble_xml_file() → (*chart->write_chart_type)() // NULL function pointer call → SEGV at 0x000000000000 ``` ## To Reproduce **Ultra-minimal reproduction code (6 APIs, tested and verified):** ```c #include <xlsxwriter.h> #include <stdio.h> int main() { printf("ULTRA MINIMAL: 6 APIs only - libxlsxwriter crash\n"); // 1. Create workbook lxw_workbook *workbook = workbook_new("test.xlsx"); if (!workbook) return 1; // 2. Add worksheet (required for chart insertion) lxw_worksheet *worksheet = workbook_add_worksheet(workbook, NULL); if (!worksheet) { workbook_close(workbook); return 1; } // 3. Create chart with INVALID type 0 - KEY TRIGGER lxw_chart *chart = workbook_add_chart(workbook, 0); if (chart) { // 4. Add series (required - chart must have series) lxw_chart_series *series = chart_add_series(chart, "", ""); // 5. Insert chart into worksheet (required to trigger processing) worksheet_insert_chart(worksheet, 0, 0, chart); } printf("Calling workbook_close - crash expected...\n"); // 6. Close workbook - TRIGGERS NULL POINTER DEREFERENCE workbook_close(workbook); printf("ERROR: Should never reach this line!\n"); return 0; } ``` ### Compilation and Testing ``` # In libxlsxwriter docker container gcc -g -fsanitize=address -I/src/libxlsxwriter/include \ ultra_minimal.c -L/src/libxlsxwriter-install/lib -lxlsxwriter -lz \ -o ultra_minimal && ./ultra_minimal ``` ## Expected behavior The library should: 1. **Validate chart types** during creation and reject invalid types with clear error codes 2. **Fail gracefully** for invalid inputs rather than creating partially initialized objects 3. **Never crash** regardless of input combination ```c static lxw_error _write_chart_files(lxw_packager *self) { // ... lxw_chart_assemble_xml_file(chart); // Calls NULL function pointer } ``` ## Version - **libxlsxwriter version**: Current git master - **OS and version**: Linux x86_64 - **Compiler**: clang with AddressSanitizer - **Build configuration**: Docker container build - **Testing**: Verified reproduction in libxlsxwriter docker environment ## Additional Context This bug was discovered through fuzz testing. By examining the code, I believe that since LXW_CHART_NONE is defined in the enum, it is inappropriate to leave it unhandled. This does not fall under API misuse. It can be addressed simply with an if check, or even with an assert. (Of course, for values outside the range of the enum provided to users, we don’t need to handle them — that would be API misuse.) Of course, there are other higher-cost approaches, but I don’t think it is appropriate to introduce significant overhead for such a minor bug. Since I am a fuzz tester and lack prior knowledge of this library, my proposed solution may not be entirely accurate. If you have other ideas, I’d be happy to discuss them.