From 156669166d19aea6b0542c2b8a65732b77d41b59 Mon Sep 17 00:00:00 2001 From: Cameron Cawley Date: Sun, 16 Jan 2022 19:47:01 +0000 Subject: [PATCH 1/5] Fix bzcat UBSan false positives for base and limit. Co-authored-by: AliceLR --- toys/other/bzcat.c | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/toys/other/bzcat.c b/toys/other/bzcat.c index f7453441e..076dfee00 100644 --- a/toys/other/bzcat.c +++ b/toys/other/bzcat.c @@ -62,7 +62,9 @@ config BZCAT // This is what we know about each huffman coding group struct group_data { - int limit[MAX_HUFCODE_BITS+1], base[MAX_HUFCODE_BITS], permute[MAX_SYMBOLS]; + // limit and base are 1-indexed. index 0 is never used but increasing + // the length by 1 simplifies the code and isn't that much of a waste. + int limit[1+MAX_HUFCODE_BITS+1], base[1+MAX_HUFCODE_BITS], permute[MAX_SYMBOLS]; char minLen, maxLen; }; @@ -161,7 +163,7 @@ static unsigned int get_bits(struct bunzip_data *bd, char bits_wanted) static int read_block_header(struct bunzip_data *bd, struct bwdata *bw) { struct group_data *hufGroup; - int hh, ii, jj, kk, symCount, *base, *limit; + int hh, ii, jj, kk, symCount; unsigned char uc; // Read in header signature and CRC (which is stored big endian) @@ -273,16 +275,10 @@ static int read_block_header(struct bunzip_data *bd, struct bwdata *bw) hufGroup->minLen = minLen; hufGroup->maxLen = maxLen; - // Note that minLen can't be smaller than 1, so we adjust the base - // and limit array pointers so we're not always wasting the first - // entry. We do this again when using them (during symbol decoding). - base = hufGroup->base-1; - limit = hufGroup->limit-1; - // zero temp[] and limit[], and calculate permute[] pp = 0; for (ii = minLen; ii <= maxLen; ii++) { - temp[ii] = limit[ii] = 0; + temp[ii] = hufGroup->limit[ii] = 0; for (hh = 0; hh < symCount; hh++) if (length[hh] == ii) hufGroup->permute[pp++] = hh; } @@ -297,13 +293,13 @@ static int read_block_header(struct bunzip_data *bd, struct bwdata *bw) pp = hh = 0; for (ii = minLen; ii < maxLen; ii++) { pp += temp[ii]; - limit[ii] = pp-1; + hufGroup->limit[ii] = pp-1; pp <<= 1; - base[ii+1] = pp-(hh+=temp[ii]); + hufGroup->base[ii+1] = pp-(hh+=temp[ii]); } - limit[maxLen] = pp+temp[maxLen]-1; - limit[maxLen+1] = INT_MAX; - base[minLen] = 0; + hufGroup->limit[maxLen] = pp+temp[maxLen]-1; + hufGroup->limit[maxLen+1] = INT_MAX; + hufGroup->base[minLen] = 0; } return 0; @@ -320,7 +316,7 @@ static int read_huffman_data(struct bunzip_data *bd, struct bwdata *bw) { struct group_data *hufGroup; int ii, jj, kk, runPos, dbufCount, symCount, selector, nextSym, - *byteCount, *base, *limit; + *byteCount; unsigned hh, *dbuf = bw->dbuf; unsigned char uc; @@ -340,7 +336,6 @@ static int read_huffman_data(struct bunzip_data *bd, struct bwdata *bw) // linearly, staying in cache more, so isn't as limited by DRAM access.) runPos = dbufCount = symCount = selector = 0; // Some unnecessary initializations to shut gcc up. - base = limit = 0; hufGroup = 0; hh = 0; @@ -351,14 +346,12 @@ static int read_huffman_data(struct bunzip_data *bd, struct bwdata *bw) symCount = GROUP_SIZE-1; if (selector >= bd->nSelectors) return RETVAL_DATA_ERROR; hufGroup = bd->groups + bd->selectors[selector++]; - base = hufGroup->base-1; - limit = hufGroup->limit-1; } // Read next huffman-coded symbol (into jj). ii = hufGroup->minLen; jj = get_bits(bd, ii); - while (jj > limit[ii]) { + while (jj > hufGroup->limit[ii]) { // if (ii > hufGroup->maxLen) return RETVAL_DATA_ERROR; ii++; @@ -369,7 +362,7 @@ static int read_huffman_data(struct bunzip_data *bd, struct bwdata *bw) jj = (jj << 1) | kk; } // Huffman decode jj into nextSym (with bounds checking) - jj-=base[ii]; + jj-=hufGroup->base[ii]; if (ii > hufGroup->maxLen || (unsigned)jj >= MAX_SYMBOLS) return RETVAL_DATA_ERROR; From 5a850bf17fe58d41a46e58edea5890ca896f2d68 Mon Sep 17 00:00:00 2001 From: Cameron Cawley Date: Sun, 16 Jan 2022 21:29:43 +0000 Subject: [PATCH 2/5] Fix "applying offset to null pointer" warnings with UBSan --- main.c | 4 +++- toys/lsb/seq.c | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/main.c b/main.c index d86a6ecd4..6b39e7111 100644 --- a/main.c +++ b/main.c @@ -143,7 +143,9 @@ void toy_init(struct toy_list *which, char *argv[]) // Free old toys contents (to be reentrant), but leave rebound if any // don't blank old optargs if our new argc lives in the old optargs. - if (argvtoys.optargs+toys.optc) free(toys.optargs); + if (toys.optargs) { + if (argvtoys.optargs+toys.optc) free(toys.optargs); + } memset(&toys, 0, offsetof(struct toy_context, rebound)); if (oldwhich) memset(&this, 0, sizeof(this)); diff --git a/toys/lsb/seq.c b/toys/lsb/seq.c index 7a931c64c..0391333ea 100644 --- a/toys/lsb/seq.c +++ b/toys/lsb/seq.c @@ -47,7 +47,10 @@ static double parsef(char *s) { char *dp = strchr(s, '.'); - if (dp++) TT.precision = maxof(TT.precision, strcspn(dp, "eE")); + if (dp) { + dp++; + TT.precision = maxof(TT.precision, strcspn(dp, "eE")); + } return xstrtod(s); } From 31c5747d3945d9c7d3003c02047e26ff80dde2c8 Mon Sep 17 00:00:00 2001 From: Cameron Cawley Date: Sun, 16 Jan 2022 21:37:11 +0000 Subject: [PATCH 3/5] Fix "index 4096 out of bounds" warnings with UBSan --- toys/other/blkid.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/toys/other/blkid.c b/toys/other/blkid.c index 01b5971b6..03fca8ed4 100644 --- a/toys/other/blkid.c +++ b/toys/other/blkid.c @@ -106,6 +106,10 @@ static void do_blkid(int fd, char *name) pass++; continue; } + if (fstypes[i].magic_len+fstypes[i].magic_offset-off > sizeof(toybuf)) { + pass++; + continue; + } if (fstypes[i].magic_offset < off) continue; // Populate 64 bit little endian magic value @@ -141,7 +145,7 @@ static void do_blkid(int fd, char *name) len = fstypes[i].label_len; if (!FLAG(U) && len) { - s = toybuf+fstypes[i].label_off-off; + s = toybuf+(fstypes[i].label_off-off); if (!strcmp(type, "vfat")) { show_tag("SEC_TYPE", "msdos"); while (len && s[len-1]==' ') len--; From 814ed78a73f0671b3df6e46bafb726fdd43bb9ec Mon Sep 17 00:00:00 2001 From: Cameron Cawley Date: Sun, 16 Jan 2022 21:43:24 +0000 Subject: [PATCH 4/5] Fix "null pointer passed as argument 1, which is declared to never be null" warning with UBSan --- toys/posix/ls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toys/posix/ls.c b/toys/posix/ls.c index 155110591..6b692f6e5 100644 --- a/toys/posix/ls.c +++ b/toys/posix/ls.c @@ -315,7 +315,7 @@ static void listfiles(int dirfd, struct dirtree *indir) if (!FLAG(f)) { unsigned long long blocks = 0; - qsort(sort, dtlen, sizeof(void *), (void *)compare); + if (dtlen > 0) qsort(sort, dtlen, sizeof(void *), (void *)compare); for (ul = 0; ul Date: Sun, 16 Jan 2022 21:51:15 +0000 Subject: [PATCH 5/5] Fix "signed integer overflow" warning with UBSan --- toys/other/bzcat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toys/other/bzcat.c b/toys/other/bzcat.c index 076dfee00..b9342ddb8 100644 --- a/toys/other/bzcat.c +++ b/toys/other/bzcat.c @@ -126,7 +126,7 @@ static unsigned int get_bits(struct bunzip_data *bd, char bits_wanted) // Avoid 32-bit overflow (dump bit buffer to top of output) if (bd->inbufBitCount>=24) { - bits = bd->inbufBits&((1<inbufBitCount)-1); + bits = bd->inbufBits&((1u<inbufBitCount)-1); bits_wanted -= bd->inbufBitCount; bits <<= bits_wanted; bd->inbufBitCount = 0;