Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: workbook_validate_sheet_name buffer-overflow #442

Closed
wxie7 opened this issue May 8, 2024 · 4 comments
Closed

Bug: workbook_validate_sheet_name buffer-overflow #442

wxie7 opened this issue May 8, 2024 · 4 comments
Assignees
Labels

Comments

@wxie7
Copy link

wxie7 commented May 8, 2024

hello, maybe there exist a bug in workbook_validate_sheet_name.
When sheetname is an empty string (""), the workbook_validate_sheet_name function does not check if the string length is 0, leading to a buffer overflow.The following is the relevant code, the crash occurs at workbook.c:workbook_validate_sheet_name.

#include "xlsxwriter.h"

int main() {

    lxw_workbook  *workbook  = workbook_new("demo.xlsx");
    lxw_worksheet *worksheet = workbook_add_worksheet(workbook, NULL);
    const char* name = "";
    lxw_error le = workbook_validate_sheet_name(workbook, name);
    if (le == LXW_NO_ERROR) {
      lxw_worksheet *worksheet = workbook_add_worksheet(workbook, name);
    }

    return 0;
}
@jmcnamara jmcnamara self-assigned this May 8, 2024
@jmcnamara jmcnamara added the bug label May 8, 2024
@jmcnamara
Copy link
Owner

Thanks for the report. That is omission/bug. I'll add a fix.

jmcnamara added a commit that referenced this issue May 8, 2024
jmcnamara added a commit that referenced this issue May 8, 2024
@jmcnamara
Copy link
Owner

I've pushed a fix for this to main. There is now a new error code called LXW_ERROR_SHEETNAME_IS_BLANK for this condition.

@wxie7
Copy link
Author

wxie7 commented May 9, 2024

Should verify in advance that name is NULL?

jmcnamara added a commit that referenced this issue May 9, 2024
@jmcnamara
Copy link
Owner

jmcnamara commented May 9, 2024

Should verify in advance that name is NULL?

My initial thought was that the end user should check for NULL and that workbook_validate_sheet_name() should validate the name and not the string. However, most libxlsxwriter functions check for NULL so I've added a LXW_ERROR_NULL_PARAMETER_IGNORED error as well.

I've force pushed that change to main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants