Fix heap-buffer-overflow in explicit_location_lex_one
authorYao Qi <yao.qi@linaro.org>
Mon, 15 Aug 2016 11:28:56 +0000 (12:28 +0100)
committerYao Qi <yao.qi@linaro.org>
Mon, 15 Aug 2016 11:28:56 +0000 (12:28 +0100)
I build GDB with -fsanitize=address, and see the error in tests,

(gdb) PASS: gdb.linespec/ls-errs.exp: lang=C++: break 3 foo
break -line 3 foo^M
=================================================================^M
==4401==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000047487 at pc 0x819d8e bp 0x7fff4e4e6bb0 sp 0x7fff4e4e6ba8^M
READ of size 1 at 0x603000047487 thread T0^[[1m^[[0m^M
    #0 0x819d8d in explicit_location_lex_one /home/yao/SourceCode/gnu/gdb/git/gdb/location.c:502^M
    #1 0x81a185 in string_to_explicit_location(char const**, language_defn const*, int) /home/yao/SourceCode/gnu/gdb/git/gdb/location.c:556^M
    #2 0x81ac10 in string_to_event_location(char**, language_defn const*) /home/yao/SourceCode/gnu/gdb/git/gdb/location.c:687^

the code in question is:

>         /* Special case: C++ operator,.  */
>         if (language->la_language == language_cplus
>             && strncmp (*inp, "operator", 8)  <--- [1]
>             && (*inp)[9] == ',')
>           (*inp) += 9;
>         ++(*inp);

The error is caused by the access to (*inp)[9] if 9 is out of its bounds.
However [1] looks odd to me, because if strncmp returns true (non-zero),
the following check "(*inp)[9] == ','" makes no sense any more.  I
suspect it was a typo in the code we meant to "strncmp () == 0".  Another
problem in the code above is that if *inp is "operator,", we first
increment *inp by 9, and then increment it by one again, which is wrong
to me.  We should only increment *inp by 8 to skip "operator", and go
back to the loop header to decide where we stop.

gdb:

2016-08-15  Yao Qi  <yao.qi@linaro.org>

* location.c (explicit_location_lex_one): Compare the return
value of strncmp with zero.  Don't check (*inp)[9].  Increment
*inp by 8.

gdb/ChangeLog
gdb/location.c

index 91e8b717bf21d2c12993c0ba232f45faa608e297..62aa1292846d49c46b43943557004e2161c4b2fd 100644 (file)
@@ -1,3 +1,9 @@
+2016-08-15  Yao Qi  <yao.qi@linaro.org>
+
+       * location.c (explicit_location_lex_one): Compare the return
+       value of strncmp with zero.  Don't check (*inp)[9].  Increment
+       *inp by 8.
+
 2016-08-11  Pedro Alves  <palves@redhat.com>
 
        PR gdb/20413
index 071d262d9f934a3a09f39f5c4e6cd465a538b8b0..65116c732f7071444986c4b582580b7cc4622f0b 100644 (file)
@@ -498,9 +498,8 @@ explicit_location_lex_one (const char **inp,
        {
          /* Special case: C++ operator,.  */
          if (language->la_language == language_cplus
-             && strncmp (*inp, "operator", 8)
-             && (*inp)[9] == ',')
-           (*inp) += 9;
+             && strncmp (*inp, "operator", 8) == 0)
+           (*inp) += 8;
          ++(*inp);
        }
     }