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 <kzak@redhat.com>

diff --git a/tests/expected/hexdump/highlighting-8b_hex-1 b/tests/expected/hexdump/highlighting-8b_hex-1
new file mode 100644 (file)
index 0000000..3de259e
--- /dev/null
+++ b/tests/expected/hexdump/highlighting-8b_hex-1
@@ -0,0 +1,2 @@
+\e[31m4847464544434241\e[0m
+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 (file)
index 0000000..5c4809b
--- /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 343cc7927f12a8b4069c1d7b58b42764b4818503..eb78b7e1d6f1fbf3dbc765a8d774af5080479b9c 100755 (executable)
--- 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 733318b5b9aa6021d6ce8f641b3fa74f3df23ef5..5f0f48bfffe52199e1516c558ea331ff06da1518 100644 (file)
--- 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 f36b27e8d6e116b4d104d014d8b56a36e8cc37d1..5da92349c3c3e75c708a848570977422d3d681e3 100644 (file)
--- 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 1268fa1c67fc2cd291fdd81b4fbbcee9355107b2..309fbee4f53f6b9abe950a19bc1ae8715cbc95dd 100644 (file)
--- 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? */
 };