Commit 33c846ef authored by Markus Armbruster's avatar Markus Armbruster
Browse files

gdbstub: Fix misuse of isxdigit() 



gdb_read_byte() passes its @ch argument to isxdigit().  Undefined
behavior when the value is negative.  Two callers:

* gdb_chr_receive() passes an uint8_t value.  Safe.

* gdb_handlesig() a char value.  Unsafe.  Not a security issue,
  because the characters come from the gdb client, which is trusted.

The obvious fix would be casting @ch to unsigned char.  But note that
gdb_read_byte() already casts @ch to uint8_t in many places.  Uses of
@ch without such a cast:

(1) Compare to a character constant with == or !=

(2) s->linesum += ch

(3) Store ch or ch ^ 0x20 into s->line_buf[]

(4) Check for invalid RLE count:
    ch < ' ' || ch == '#' || ch == '$' || ch > 126

(5) Pass to isxdigit()

(6) Pass to fromhex()

Change the parameter type from int to uint8_t, and drop the now
redundant casts.  Affects the above uses as follows:

(1) No change: the character constants are all non-negative.

(2) Effectively no change: we only ever use s->linesum & 0xff, and
    s->linesum is int.

(3) No change: s->line_buf[] is char[].

(4) No change.

(5) Avoid undefined behavior.

(6) No change: only reached when isxdigit(ch)

Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
Message-Id: <20190514180311.16028-5-armbru@redhat.com>
parent 046aba16

gdbstub.c

+7 −7
Original line number Diff line number Diff line
@@ -1987,7 +1987,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...) 
    va_end(va);

}



static void gdb_read_byte(GDBState *s, int ch)

static void gdb_read_byte(GDBState *s, uint8_t ch)

{

    uint8_t reply;
@@ -2001,7 +2001,7 @@ static void gdb_read_byte(GDBState *s, int ch) 
        } else if (ch == '+') {

            trace_gdbstub_io_got_ack();

        } else {

            trace_gdbstub_io_got_unexpected((uint8_t)ch);

            trace_gdbstub_io_got_unexpected(ch);

        }



        if (ch == '+' || ch == '$')
@@ -2024,7 +2024,7 @@ static void gdb_read_byte(GDBState *s, int ch) 
                s->line_sum = 0;

                s->state = RS_GETLINE;

            } else {

                trace_gdbstub_err_garbage((uint8_t)ch);

                trace_gdbstub_err_garbage(ch);

            }

            break;

        case RS_GETLINE:
@@ -2070,11 +2070,11 @@ static void gdb_read_byte(GDBState *s, int ch) 
             */

            if (ch < ' ' || ch == '#' || ch == '$' || ch > 126) {

                /* invalid RLE count encoding */

                trace_gdbstub_err_invalid_repeat((uint8_t)ch);

                trace_gdbstub_err_invalid_repeat(ch);

                s->state = RS_GETLINE;

            } else {

                /* decode repeat length */

                int repeat = (unsigned char)ch - ' ' + 3;

                int repeat = ch - ' ' + 3;

                if (s->line_buf_index + repeat >= sizeof(s->line_buf) - 1) {

                    /* that many repeats would overrun the command buffer */

                    trace_gdbstub_err_overrun();
@@ -2096,7 +2096,7 @@ static void gdb_read_byte(GDBState *s, int ch) 
        case RS_CHKSUM1:

            /* get high hex digit of checksum */

            if (!isxdigit(ch)) {

                trace_gdbstub_err_checksum_invalid((uint8_t)ch);

                trace_gdbstub_err_checksum_invalid(ch);

                s->state = RS_GETLINE;

                break;

            }
@@ -2107,7 +2107,7 @@ static void gdb_read_byte(GDBState *s, int ch) 
        case RS_CHKSUM2:

            /* get low hex digit of checksum */

            if (!isxdigit(ch)) {

                trace_gdbstub_err_checksum_invalid((uint8_t)ch);

                trace_gdbstub_err_checksum_invalid(ch);

                s->state = RS_GETLINE;

                break;

            }