Skip to content

Commit

Permalink
Fix sanitizer warning (#771)
Browse files Browse the repository at this point in the history
* Let's try this

* Fix out-of-bounds access in MULRK struct handling

* Take 2

In response to seeing this:

libxls/xls.c:589:23: runtime error: load of misaligned address 0x50700005cbb6 for type 'DWORD', which requires 4 byte alignment

* Move and turn into inline functions

The use of inline functions is in response to this:

Found the following significant warnings:
  libxls/xls.c:121:31: warning: ISO C forbids braced-groups within expressions [-Wpedantic]
  libxls/xls.c:126:34: warning: ISO C forbids braced-groups within expressions [-Wpedantic]

* Apply fix to another MULRK location

* Revert "Apply fix to another MULRK location"

This reverts commit 738df8a.

* Craft a similar fix for MULBLANK (not MULRK, oops)

* Fix for access inside LABEL

* Adopt patch strategy used in positron

* Revert whitespace changes I did not mean to make earlier

* Moar checks

* Get rid of trailing comma
  • Loading branch information
jennybc authored Mar 7, 2025
1 parent f070f74 commit 9e20981
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 5 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/rhub.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ on:
config:
description: 'A comma separated list of R-hub platforms to use. These default choices have been customized for readxl.'
type: string
default: 'gcc-asan,valgrind,rchk,gcc15'
default: 'gcc-asan,valgrind,rchk,gcc15,clang-asan,clang-ubsan'
name:
description: 'Run name. You can leave this empty now.'
type: string
Expand Down
15 changes: 11 additions & 4 deletions src/libxls/xls.c
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,10 @@ int xls_isCellTooSmall(xlsWorkBook* pWB, BOF* bof, BYTE* buf) {
if (bof->size < offsetof(LABEL, value) + 2)
return 1;

size_t label_len = ((LABEL*)buf)->value[0] + (((LABEL*)buf)->value[1] << 8);
// --- Start readxl ---
BYTE *value = get_LABEL_value((LABEL*)buf);
size_t label_len = value[0] + (value[1] << 8);
// --- End readxl ---
if (pWB->is5ver) {
return (bof->size < offsetof(LABEL, value) + 2 + label_len);
}
Expand Down Expand Up @@ -580,8 +583,10 @@ static struct st_cell_data *xls_addCell(xlsWorkSheet* pWS,BOF* bof,BYTE* buf)
}
cell=&row->cells.cell[index];
cell->id=XLS_RECORD_RK;
cell->xf=xlsShortVal(((MULRK*)buf)->rk[i].xf);
cell->d=NumFromRk(xlsIntVal(((MULRK*)buf)->rk[i].value));
// --- Start readxl ---
cell->xf=xlsShortVal(get_MULRK_RK_XF((MULRK*)buf, i));
cell->d=NumFromRk(xlsIntVal(get_MULRK_RK_VALUE((MULRK*)buf, i)));
// --- End readxl ---
xls_cell_set_str(cell, xls_getfcell(pWS->workbook,cell, NULL));
}
break;
Expand All @@ -595,7 +600,9 @@ static struct st_cell_data *xls_addCell(xlsWorkSheet* pWS,BOF* bof,BYTE* buf)
}
cell=&row->cells.cell[index];
cell->id=XLS_RECORD_BLANK;
cell->xf=xlsShortVal(((MULBLANK*)buf)->xf[i]);
// --- Start readxl ---
cell->xf=xlsShortVal(get_MULBLANK_XF((MULBLANK*)buf, i));
// --- End readxl ---
xls_cell_set_str(cell, xls_getfcell(pWS->workbook,cell, NULL));
}
break;
Expand Down
30 changes: 30 additions & 0 deletions src/libxls/xlsstruct.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
#define XLS_STRUCT_INC

#include "libxls/ole.h"
// --- Start readxl ---
#include <string.h>
// --- End readxl ---

#define XLS_RECORD_EOF 0x000A
#define XLS_RECORD_DEFINEDNAME 0x0018
Expand Down Expand Up @@ -204,6 +207,19 @@ typedef struct MULRK
}
MULRK;

// --- Start readxl ---
static inline WORD get_MULRK_RK_XF(MULRK *mulrk, int i) {
WORD xf;
memcpy(&xf, (BYTE *)mulrk + offsetof(MULRK, rk) + i * (sizeof(WORD) + sizeof(DWORD)), sizeof(WORD));
return xf;
}

static inline DWORD get_MULRK_RK_VALUE(MULRK *mulrk, int i) {
DWORD value;
memcpy(&value, (BYTE *)mulrk + offsetof(MULRK, rk) + i * (sizeof(WORD) + sizeof(DWORD)) + sizeof(WORD), sizeof(DWORD));
return value;
}
// --- End readxl ---
typedef struct MULBLANK
{
WORD row;
Expand All @@ -213,6 +229,14 @@ typedef struct MULBLANK
}
MULBLANK;

// --- Start readxl ---
static inline WORD get_MULBLANK_XF(MULBLANK *mulblank, int i) {
WORD xf;
memcpy(&xf, (BYTE *)mulblank + offsetof(MULBLANK, xf) + i * sizeof(WORD), sizeof(WORD));
return xf;
}
// --- End readxl ---

typedef struct BLANK
{
WORD row;
Expand All @@ -230,6 +254,12 @@ typedef struct LABEL
}
LABEL;

// --- Start readxl ---
static inline BYTE *get_LABEL_value(LABEL *label) {
return (BYTE *)label + offsetof(LABEL, value);
}
// --- End readxl ---

typedef struct BOOLERR
{
WORD row;
Expand Down

0 comments on commit 9e20981

Please sign in to comment.