PATCH: libobjc/9751

[Nicola Pero <nicola at brainstorm dot co dot uk>]

This patch fixes liblibobjc/9751 - the bug reported unsafe management of
strings during some libobjc type name manipulations when running under GC.

See the discussion at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=9751

The included patch fixes the issues discussed, plus similar issues a few
lines later.

To test the patch properly, I didn' think writing a testcase was a good
idea.  Not only it's difficult to trigger the bug (it only happens when a
small malloc() returns dirty memory), but we don't test libobjc with GC,
and the code is not even used without GC.

So I extracted the code and created a small program which performs the
string manipulations.  If you run the code after the patch, you see it
works.  If you run it before the patch, you see it still works ... but
only because malloc() returns by chance memory initialized to 0!  If you
insert manually code filling for example the buffer with 'A' just after
malloc() (to simulate malloc() returning dirty memory), then the new code
works, but the old one fails (as expected).  This (uninteresting/trivial)
testing machinery in attach.

Ok to apply ?


Mon Jul 21 13:29:23 2003  Nicola Pero  <nicola@brainstorm.co.uk>

	libobjc/9751
	* gc.c (class_ivar_set_gcinvisible): Fixed unsafe string
	manipulation.

Index: gc.c
===================================================================
RCS file: /cvs/gcc/gcc/libobjc/gc.c,v
retrieving revision 1.6
diff -u -r1.6 gc.c
--- gc.c	23 May 2003 20:04:58 -0000	1.6
+++ gc.c	21 Jul 2003 12:19:53 -0000
@@ -405,34 +405,69 @@
 
       if (*type == _C_GCINVISIBLE)
 	{
-	  char *new_type;
-
 	  if (gc_invisible || ! __objc_ivar_pointer (type))
 	    return;	/* The type of the variable already matches the
 			   requested gc_invisible type */
-
-	  /* The variable is gc_invisible and we have to reverse it */
-	  new_type = objc_atomic_malloc (strlen (ivar->ivar_type));
-	  strncpy (new_type, ivar->ivar_type,
-		   (size_t)(type - ivar->ivar_type));
-	  strcat (new_type, type + 1);
-	  ivar->ivar_type = new_type;
+	  else
+	    {
+	      /* Create a new_type string which is the same as the
+	       * original ivar->ivar_type string, but with the
+	       * _C_GCINVISIBLE ('!') character removed.
+	       */
+	      char *new_type;
+	      size_t name_length = type - ivar->ivar_type;
+	      size_t total_length = strlen (ivar->ivar_type);
+	      
+	      /* Allocate space for the new type.  Please note that we
+	       * allocate 1 byte less than the memory taken by the
+	       * original type.  */
+	      new_type = objc_atomic_malloc (total_length);
+	      
+	      /* Copy variable name in the first part of new_type.  */
+	      memcpy (new_type, ivar->ivar_type, name_length);
+	      
+	      /* Copy the other type info, excluding the first '!'
+	       * character (but including the '\0' terminator), in the
+	       * second part of new type.  */
+	      memcpy (new_type + name_length, type + 1, 
+		      total_length - name_length);
+	      
+	      ivar->ivar_type = new_type;
+	    }
 	}
       else
 	{
-	  char *new_type;
-
 	  if (! gc_invisible || ! __objc_ivar_pointer (type))
 	    return;	/* The type of the variable already matches the
 			   requested gc_invisible type */
+	  else
+	    {
+	      /* Create a new_type string which is the same as the
+	       * original ivar->ivar_type string, but with the
+	       * _C_GCINVISIBLE ('!') character added.
+	       */
+	      char *new_type;
+	      size_t name_length = type - ivar->ivar_type;
+	      size_t total_length = strlen (ivar->ivar_type);
+	      
+	      /* Allocate space for the new type.  Please note that we
+	       * allocate 1 byte more than the memory taken by the
+	       * original type.  */
+	      new_type = objc_atomic_malloc (total_length + 2);
+	      
+	      /* Copy variable name in the first part of new_type.  */
+	      memcpy (new_type, ivar->ivar_type, name_length);
+	      
+	      /* Add a '!' character.  */
+	      memcpy (new_type + name_length, "!", 1);
 
-	  /* The variable is gc visible and we have to make it gc_invisible */
-	  new_type = objc_malloc (strlen (ivar->ivar_type) + 2);
-	  strncpy (new_type, ivar->ivar_type,
-		   (size_t)(type - ivar->ivar_type));
-	  strcat (new_type, "!");
-	  strcat (new_type, type);
-	  ivar->ivar_type = new_type;
+	      /* Copy the other type info, (including the '\0'
+	       * terminator), in the second part of new type.  */
+	      memcpy (new_type + name_length + 1, type, 
+		      total_length - name_length + 1);
+	      
+	      ivar->ivar_type = new_type;
+	    }
 	}
 
       __objc_generate_gc_type_description (class);

Attachment: ShortTest.tgz
Description: GNU Zip compressed data