[Bug gdb/11092] New: c_printstr in c-lang.c length parameter can overflow

[Bug gdb/11092] New: c_printstr in c-lang.c length parameter can overflow

[pmuldoon at redhat dot com]

I noticed this bug while working on another patch.  Comments and documentation
refer to the c_printstr function allowing a length of -1.  But the length
parameter in c_printstr is of type: unsigned int.  There is logic in the
function to work with the length parameter being negative, so this just seems
like a regression.  Supplying a length of -1 will overflow the usigned int,
causing a very large length. This normally results in a sigsegv later in the
function

-- 
           Summary: c_printstr in c-lang.c length parameter can overflow
           Product: gdb
           Version: 7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: gdb
        AssignedTo: unassigned at sourceware dot org
        ReportedBy: pmuldoon at redhat dot com
                CC: gdb-prs at sourceware dot org


http://sourceware.org/bugzilla/show_bug.cgi?id=11092

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug gdb/11092] c_printstr in c-lang.c length parameter can overflow

[schwab at linux-m68k dot org]

------- Additional Comments From schwab at linux-m68k dot org  2009-12-15 15:15 -------
The function correctly checks for length == -1.  Unsigned variables cannot
overflow.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID


http://sourceware.org/bugzilla/show_bug.cgi?id=11092

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug gdb/11092] c_printstr in c-lang.c length parameter can overflow

[pmuldoon at redhat dot com]

------- Additional Comments From pmuldoon at redhat dot com  2009-12-15 15:37 -------
Maybe I misunderstood the comments that head that function. Can I not  provide
-1 as a length to terminate string printing at the first null of appropriate
width? If I do that I get the outcome below: (ignore the extra encoding argument
to LA_PRINT_STRING, it is part of my patch and has no effect on the outcome):

I've redacted the output from the debugging session to show in summary:

Breakpoint 1, main () at
../../../archer/gdb/testsuite/gdb.python/py-prettyprint.c:252
252	  return 0;      /* break to inspect struct and union */
Current language:  auto
(gdb) p estring
Breakpoint 3, print_string_repr (printer=0x7ffff20dd7e8, hint=0xf4f6e0 "string",
stream=0xbc1ab0, recurse=0, options=0x7fffffffd8e0, language=0x7ab4c0,
gdbarch=0xc20880)
    at ../../archer/gdb/python/py-prettyprint.c:323
323		LA_PRINT_STRING (stream, builtin_type (gdbarch)->builtin_char,

(top-gdb) list 323
323		LA_PRINT_STRING (stream, builtin_type (gdbarch)->builtin_char,
324				 output, length, encoding, 0, options);
325	      else
326		fputs_filtered (output, stream);
327	

(top-gdb) p length
$1 = -1
(top-gdb) p output
$2 = (gdb_byte *) 0xdfd530 "embedded x\201\202\203\204"

(top-gdb) s
c_printstr (stream=0xbc1ab0, type=0xc10760, string=0xdfd530 "embedded
x\201\202\203\204", length=4294967295, user_encoding=0xf71340 "UTF-8",
force_ellipses=0, options=0x7fffffffd8e0)
    at ../../archer/gdb/c-lang.c:375
375	  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
(top-gdb) n
377	  unsigned int things_printed = 0;
(top-gdb) 
378	  int in_quotes = 0;
(top-gdb) 
379	  int need_comma = 0;
(top-gdb) 
380	  int width = TYPE_LENGTH (type);
(top-gdb) 
387	  int finished = 0;
(top-gdb) 
388	  int need_escape = 0;
(top-gdb) 
393	  if (!force_ellipses
(top-gdb) 
395	      && (extract_unsigned_integer (string + (length - 1) * width,
(top-gdb) p length
$3 = 4294967295
(top-gdb) n

Program received signal SIGSEGV, Segmentation fault.
0x00000000004f8180 in extract_unsigned_integer (addr=0x100dfd52e <Address
0x100dfd52e out of bounds>, len=1, byte_order=BFD_ENDIAN_LITTLE) at
../../archer/gdb/findvar.c:110
110		retval = (retval << 8) | *p;


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|INVALID                     |


http://sourceware.org/bugzilla/show_bug.cgi?id=11092

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug gdb/11092] c_printstr in c-lang.c length parameter can overflow

[schwab at linux-m68k dot org]

------- Additional Comments From schwab at linux-m68k dot org  2009-12-15 16:37 -------
You're right, I missed that.  This patch should fix that:

--- c-lang.c.~1.78.~	2009-09-28 11:42:34.000000000 +0200
+++ c-lang.c	2009-12-15 17:27:21.000000000 +0100
@@ -386,6 +386,18 @@ c_printstr (struct ui_file *stream, stru
   int finished = 0;
   int need_escape = 0;
 
+  if (length == -1)
+    {
+      unsigned long current_char = 1;
+      for (i = 0; current_char; ++i)
+	{
+	  QUIT;
+	  current_char = extract_unsigned_integer (string + i * width,
+						   width, byte_order);
+	}
+      length = i;
+    }
+
   /* If the string was not truncated due to `set print elements', and
      the last byte of it is a null, we don't print that, in traditional C
      style.  */
@@ -417,18 +429,6 @@ c_printstr (struct ui_file *stream, stru
       return;
     }
 
-  if (length == -1)
-    {
-      unsigned long current_char = 1;
-      for (i = 0; current_char; ++i)
-	{
-	  QUIT;
-	  current_char = extract_unsigned_integer (string + i * width,
-						   width, byte_order);
-	}
-      length = i;
-    }
-
   /* Arrange to iterate over the characters, in wchar_t form.  */
   iter = make_wchar_iterator (string, length * width, encoding, width);
   cleanup = make_cleanup_wchar_iterator (iter);


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11092

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug gdb/11092] c_printstr in c-lang.c length parameter can overflow

[pmuldoon at redhat dot com]

------- Additional Comments From pmuldoon at redhat dot com  2009-12-16 08:23 -------
Thanks for the speedy fix.  I tested this morning, and the fix works.  Passing
in a length of -1 will now terminate string printing at the first null of
appropriate width. 

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1


http://sourceware.org/bugzilla/show_bug.cgi?id=11092

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug gdb/11092] c_printstr in c-lang.c length parameter can overflow

[schwab at linux-m68k dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2010-05-17 16:53 -------
Subject: Bug 11092

CVSROOT:	/cvs/src
Module name:	src
Changes by:	schwab@sourceware.org	2010-05-17 16:53:21

Modified files:
	gdb            : ChangeLog c-lang.c 

Log message:
	PR gdb/11092
	* c-lang.c (c_printstr): Compute real length of NUL terminated
	string at first.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.11811&r2=1.11812
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/c-lang.c.diff?cvsroot=src&r1=1.85&r2=1.86


------- Additional Comments From schwab at linux-m68k dot org  2010-05-17 16:54 -------
Fixed.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


http://sourceware.org/bugzilla/show_bug.cgi?id=11092

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.