Skip to content
  • Markus Armbruster's avatar
    33c846ef
    gdbstub: Fix misuse of isxdigit() · 33c846ef
    Markus Armbruster authored
    
    
    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>
    33c846ef
    gdbstub: Fix misuse of isxdigit()
    Markus Armbruster authored
    
    
    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>
Loading