From: Karel Zak Date: Thu, 28 May 2026 14:09:20 +0000 (+0200) Subject: hexdump: fix buffer overflow in color_cond() X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=9dbbd28223d6e5e2e1e4a949d1db6492e742be5d;p=thirdparty%2Futil-linux.git hexdump: fix buffer overflow in color_cond() Widen the color condition value from int (4 bytes) to int64_t (8 bytes) to accommodate format strings with 8-byte conversion units (e.g., 1/8 "%016x"). The memcpy() in color_cond() copies clr->range bytes into a local variable, and for 8-byte units this overflows a 4-byte int. Also switch strtoul() to strtoll() in the color format parser to correctly parse 64-bit values into the widened int64_t field. Change hexdump_clr.range from int to size_t (a byte count should never be negative), add a defensive guard against memcpy overflow in color_cond(), and add an 8-byte color condition regression test. Reported-by: Michał Majchrowicz (AFINE Team) Reported-by: Marcin Wyczechowski (AFINE Team) Signed-off-by: Karel Zak --- diff --git a/tests/expected/hexdump/highlighting-8b_hex-1 b/tests/expected/hexdump/highlighting-8b_hex-1 new file mode 100644 index 0000000000..3de259e214 --- /dev/null +++ b/tests/expected/hexdump/highlighting-8b_hex-1 @@ -0,0 +1,2 @@ +4847464544434241 +0000008 diff --git a/tests/expected/hexdump/highlighting-8b_hex-1.BE b/tests/expected/hexdump/highlighting-8b_hex-1.BE new file mode 100644 index 0000000000..5c4809b414 --- /dev/null +++ b/tests/expected/hexdump/highlighting-8b_hex-1.BE @@ -0,0 +1,2 @@ +4142434445464748 +0000008 diff --git a/tests/ts/hexdump/highlighting b/tests/ts/hexdump/highlighting index 343cc7927f..eb78b7e1d6 100755 --- a/tests/ts/hexdump/highlighting +++ b/tests/ts/hexdump/highlighting @@ -264,4 +264,13 @@ $TS_CMD_HEXDUMP $OPTS $ADDRFMT \ &> "$TS_OUTPUT" <<< "@@@@" ts_finalize_subtest +# 8-byte value match (verifies int64_t color condition) +ts_init_subtest "8b_hex-1" +TS_EXPECTED+=$BE_EXT +printf '\x41\x42\x43\x44\x45\x46\x47\x48' | \ +$TS_CMD_HEXDUMP $OPTS $ADDRFMT \ + -e '1/8 "%016x_L[red:0x4847464544434241]\n"' \ + &> "$TS_OUTPUT" +ts_finalize_subtest + ts_finalize diff --git a/text-utils/hexdump-display.c b/text-utils/hexdump-display.c index 733318b5b9..5f0f48bfff 100644 --- a/text-utils/hexdump-display.c +++ b/text-utils/hexdump-display.c @@ -227,7 +227,7 @@ static const char *color_cond(struct hexdump_pr *pr, unsigned char *bp, int bcnt /* no offset or offset outside this print unit */ if (offt < 0) offt = address; - if (offt < address || offt + clr->range > address + bcnt) + if (offt < address || offt + (off_t) clr->range > address + bcnt) continue; /* match a string */ @@ -240,13 +240,13 @@ static const char *color_cond(struct hexdump_pr *pr, unsigned char *bp, int bcnt match = 1; /* match a value */ } else if (clr->val != -1) { - int val = 0; + int64_t val = 0; /* addresses are not part of the input, so we can't * compare with the contents of bp */ if (pr->flags == F_ADDRESS) { if (clr->val == address) match = 1; - } else { + } else if (clr->range <= sizeof(val)) { memcpy(&val, bp + offt - address, clr->range); if (val == clr->val) match = 1; diff --git a/text-utils/hexdump-parse.c b/text-utils/hexdump-parse.c index f36b27e8d6..5da92349c3 100644 --- a/text-utils/hexdump-parse.c +++ b/text-utils/hexdump-parse.c @@ -512,9 +512,9 @@ static struct list_head *color_fmt(char *cfmt, int bcnt) errno = 0; end = NULL; if (cfmt[1] == 'x' || cfmt[1] == 'X') - hcnext->val = strtoul(cfmt + 2, &end, 16); + hcnext->val = strtoll(cfmt + 2, &end, 16); else - hcnext->val = strtoul(cfmt, &end, 8); + hcnext->val = strtoll(cfmt, &end, 8); if (errno || end == cfmt) badfmt(fmt); cfmt = end; @@ -561,16 +561,15 @@ static struct list_head *color_fmt(char *cfmt, int bcnt) ++cfmt; errno = 0; - hcnext->range = - strtoul(cfmt, &cfmt, 10) - hcnext->offt + 1; + unsigned long end_offt = strtoul(cfmt, &cfmt, 10); if (errno) badfmt(fmt); - /* offset range must be between 0 and format byte count */ - if (hcnext->range < 0) + if (end_offt < (unsigned long) hcnext->offt) badcnt("_L"); + hcnext->range = end_offt - hcnext->offt + 1; /* the offset extends over several print units, clone * the condition, link it in and adjust the address/offset */ - while (hcnext->range > bcnt) { + while (hcnext->range > (size_t) bcnt) { hc = xcalloc(1, sizeof(struct hexdump_clr)); memcpy(hc, hcnext, sizeof(struct hexdump_clr)); @@ -588,7 +587,7 @@ static struct list_head *color_fmt(char *cfmt, int bcnt) hcnext->offt = (off_t)-1; /* check if the string we're looking for is the same length as the range */ - if (hcnext->str && (int)strlen(hcnext->str) != hcnext->range) + if (hcnext->str && strlen(hcnext->str) != hcnext->range) badcnt("_L"); /* link in another condition */ diff --git a/text-utils/hexdump.h b/text-utils/hexdump.h index 1268fa1c67..309fbee4f5 100644 --- a/text-utils/hexdump.h +++ b/text-utils/hexdump.h @@ -39,8 +39,8 @@ struct hexdump_clr { struct list_head colorlist; /* next color unit */ const char *fmt; /* the color, UL_COLOR_* */ off_t offt; /* offset of range where unit is valid... */ - int range; /* ... and range length */ - int val; /* value ... */ + size_t range; /* ... and range length */ + int64_t val; /* value ... */ char *str; /* ... or string to match */ int invert; /* invert condition? */ };