Re: Glib proposal: add strlcpy and strlcat to glib - new version

["David Wheeler" <dwheeler ida org>]

Okay, here's an updated patch to add "strlcpy" and "strlcat" to glib,
based on Tim Janik's recommendations on June 12.  This new patch should
now be GTK+ style friendly :-).

I will say that I think "size_t" should be used instead of "gsize".
Users will essentially always pass sizeof() results as the last parameter of
these functions, and the ANSI C standard clearly defines this type as "size_t".
Using "gsize", where the standard says the type is "size_t", seems
to me to be a gratuitous incompatibility.  It's also incompatible with
the type used in strlcpy/strlcat on OpenBSD/FreeBSD/Solaris.
I suspect this is more than just the appearance of incompatibility: Aren't
there architectures where sizeof(gsize) != sizeof(size_t)?   The Glib docs say
that gsize is always guint32, but I see no reason that size_t must be the same.
If so, for very large buffers where sizeof(size_t) > 4, this could cause
a serious problem. Think 64-bit and 128-bit (!) architectures.

However, I'll grant you that in the overwhelming number of cases, if you're
using fixed-size buffers, the buffer sizes are far smaller than where
this would become an issue.  So, I'm submitting this patch using gsize
(as requested).  This can be easily switched later in the glib source, and
I wouldn't expect this to be a source compatibility problem for applications
if it were switched later.

'Nuf about gsize.  Here's a proposed ChangeLog entry, followed by the
updated (improved?) patch.



2000-06-14  David A. Wheeler <dwheeler@dwheeler.com>

	* glib.h, gstrfuncs.c: added g_strlcpy and g_strlcat to support
	  safe manipulation of fixed-length string buffers.
	  These functions were originally developed by Todd Miller to simplify
	  development of security-related programs, and
	  are available on many (but not all) Unix-like systems,
	  including OpenBSD, FreeBSD, and Solaris.  See
	  ftp://ftp.openbsd.org/pub/OpenBSD/src/lib/libc/string/strlcpy.3
	  and http://www.openbsd.org/security.html.
	  If there's a strlcpy/strlcat on the system, it's called, otherwise
	  an implementation is provided.

	* testglib.c: Added tests for g_strlcpy, g_strlcat.




The patch:  (cd glib; patch -p1 < THISFILE):


--- glib.orig/gstrfuncs.c	Wed Jun 14 00:02:09 2000
+++ glib/gstrfuncs.c	Wed Jun 14 01:35:35 2000
@@ -38,6 +38,7 @@
 #include <string.h>
 #include <locale.h>
 #include <ctype.h>		/* For tolower() */
+#include <stddef.h>             /* For size_t */
 #if !defined (HAVE_STRSIGNAL) || !defined(NO_SYS_SIGLIST_DECL)
 #include <signal.h>
 #endif
@@ -1348,6 +1349,143 @@

   return string;
 }
+
+/*
+ * Functions g_strlcpy and g_strlcat were originally developed by
+ * Todd C. Miller <Todd.Miller@courtesan.com> to simplify writing secure code.
+ * See ftp://ftp.openbsd.org/pub/OpenBSD/src/lib/libc/string/strlcpy.3
+ * for more information.
+ * Note: the original interface uses size_t, not gsize for the type of
+ * dest_size; if gsize is deprecated, switch it back to size_t.
+ * This is particularly an issue if sizeof(gsize) < sizeof(size_t),
+ * a possibility on 64-bit+ architectures if sizeof(gsize) is set to 32 bits.
+ * In such cases, these functions won't be able to use very large buffers
+ * simply because the type "gsize" is insufficiently large.
+ */
+
+/* Use the native ones, if available; they might be implemented in assembly */
+
+#ifdef HAVE_STRLCPY
+
+gsize
+g_strlcpy (gchar *dest,
+           const gchar *src,
+           gsize dest_size)
+{
+  g_return_val_if_fail( dest != NULL, 0);
+  g_return_val_if_fail( src  != NULL, 0);
+  return strlcpy(dest, src, (size_t) dest_size);
+}
+
+gsize
+g_strlcat (gchar *dest,
+           const gchar *src,
+           gsize dest_size)
+{
+  g_return_val_if_fail( dest != NULL, 0);
+  g_return_val_if_fail( src  != NULL, 0);
+  return strlcat(dest, src, (size_t) dest_size);
+}
+
+#else /* ! HAVE_STRLCPY */
+
+/*
+ * g_strlcpy
+ *
+ * Copy string src to buffer dest (of buffer size dest_size).  At most
+ * dest_size-1 characters will be copied.  Always NUL terminates
+ * (unless siz == 0).  This function does NOT allocate memory.
+ * Unlike strncpy, this function doesn't pad dest (so it's often faster).
+ * Returns size of attempted result, strlen(src),
+ * so if retval >= dest_size, truncation occurred.
+ */
+
+gsize
+g_strlcpy (gchar *dest,
+           const gchar *src,
+           gsize dest_size)
+{
+  register gchar *d = dest;
+  register const gchar *s = src;
+  register size_t n = dest_size;
+
+  g_return_val_if_fail( dest != NULL, 0);
+  g_return_val_if_fail( src  != NULL, 0);
+
+  /* Copy as many bytes as will fit */
+  if (n != 0 && --n != 0) {
+    register gchar c;
+    do
+      {
+        c = *d++ = *s++;
+        if (c == '\0')
+          break;
+      }
+    while (--n != 0);
+  }
+
+  /* If not enough room in dest, add NUL and traverse rest of src */
+  if (n == 0) {
+    if (dest_size != 0)
+      *d = '\0';    /* NUL-terminate dst */
+    while (*s++)
+      ;
+  }
+
+  return s - src - 1;  /* count does not include NUL */
+}
+
+
+/*
+ * g_strlcat
+ *
+ * Appends string src to buffer dest (of buffer size dest_size).
+ * At most dest_size-1 characters will be copied.
+ * Unlike strncat, dest_size is the full size of dest, not the space left
over.
+ * This function does NOT allocate memory.
+ * This always NUL terminates (unless siz == 0 or there were no NUL characters
+ * in the dest_size characters of dest to start with).
+ * Returns size of attempted result, which is
+ * MIN(siz,strlen(original dest)) + strlen(src),
+ * so if retval >= dest_size, truncation occurred.
+ */
+
+gsize
+g_strlcat (gchar *dest,
+           const gchar *src,
+           gsize dest_size)
+{
+  register gchar *d = dest;
+  register const gchar *s = src;
+  register gsize bytes_left = dest_size;
+  gsize dlength;  /* Logically, min(strlen(d), dest_size) */
+
+  g_return_val_if_fail( dest != NULL, 0);
+  g_return_val_if_fail( src  != NULL, 0);
+
+  /* Find the end of dst and adjust bytes left but don't go past end */
+  while (*d != '\0' && bytes_left-- != 0)
+    d++;
+  dlength = d - dest;
+  bytes_left = dest_size - dlength;
+
+  if (bytes_left == 0)
+    return(dlength + strlen(s));
+  while (*s != '\0') {
+    if (bytes_left != 1) {
+      *d++ = *s;
+      bytes_left--;
+    }
+    s++;
+  }
+  *d = '\0';
+
+  return dlength + (s - src);  /* count does not include NUL */
+}
+
+#endif /* ! HAVE_STRLCPY */
+
+

 gchar**
 g_strsplit (const gchar *string,

--- glib.orig/glib.h	Wed Jun 14 00:02:09 2000
+++ glib/glib.h	Wed Jun 14 00:43:25 2000
@@ -1611,6 +1611,14 @@
 /* removes leading & trailing spaces */
 #define g_strstrip( string )	g_strchomp (g_strchug (string))

+gsize	 g_strlcpy		(gchar *dest,
+				 const gchar *src,
+				 gsize dest_size);
+gsize	 g_strlcat		(gchar *dest,
+				 const gchar *src,
+				 gsize dest_size);
+
+
 /* String utility functions that return a newly allocated string which
  * ought to be freed with g_free from the caller at some point.
  */

--- glib.orig/testglib.c	Wed Jun 14 00:02:09 2000
+++ glib/testglib.c	Wed Jun 14 01:09:05 2000
@@ -18,7 +18,7 @@
  */

 /*
- * Modified by the GLib Team and others 1997-1999.  See the AUTHORS
+ * Modified by the GLib Team and others 1997-2000.  See the AUTHORS
  * file for a list of people on the GLib Team.  See the ChangeLog
  * files for a list of changes.  These files are distributed with
  * GLib at ftp://ftp.gtk.org/pub/gtk/.
@@ -28,6 +28,10 @@

 #include <stdio.h>
 #include <string.h>
+
+/* For size_t: */
+#include <stddef.h>
+
 #include "glib.h"

 int array[10000];
@@ -852,6 +856,63 @@
   g_free(string);

   g_print ("ok\n");
+
+  g_print("checking g_strlcpy/g_strlcat...");
+  /* The following is a torture test for strlcpy/strlcat, with lots of
+   * checking; normal users wouldn't use them this way! */
+  string = g_malloc(6);
+  *(string + 5) = 'Z'; /* guard value, shouldn't change during test */
+  *string = 'q';
+  g_assert (g_strlcpy(string, "" , 5) == 0);
+  g_assert ( *string == '\0' );
+  *string = 'q';
+  g_assert (g_strlcpy(string, "abc" , 5) == 3);
+  g_assert ( *(string + 3) == '\0' );
+  g_assert (g_str_equal(string, "abc"));
+  g_assert (g_strlcpy(string, "abcd" , 5) == 4);
+  g_assert ( *(string + 4) == '\0' );
+  g_assert ( *(string + 5) == 'Z' );
+  g_assert (g_str_equal(string, "abcd"));
+  g_assert (g_strlcpy(string, "abcde" , 5) == 5);
+  g_assert ( *(string + 4) == '\0' );
+  g_assert ( *(string + 5) == 'Z' );
+  g_assert (g_str_equal(string, "abcd"));
+  g_assert (g_strlcpy(string, "abcdef" , 5) == 6);
+  g_assert ( *(string + 4) == '\0' );
+  g_assert ( *(string + 5) == 'Z' );
+  g_assert (g_str_equal(string, "abcd"));
+  *string = 'Y';
+  *(string + 1)= '\0';
+  g_assert (g_strlcpy(string, "Hello" , 0) == 5);
+  g_assert (*string == 'Y');
+
+  *string = '\0';
+  g_assert (g_strlcat(string, "123" , 5) == 3);
+  g_assert ( *(string + 3) == '\0' );
+  g_assert (g_str_equal(string, "123"));
+  g_assert (g_strlcat(string, "" , 5) == 3);
+  g_assert ( *(string + 3) == '\0' );
+  g_assert (g_str_equal(string, "123"));
+  g_assert (g_strlcat(string, "4", 5) == 4);
+  g_assert (g_str_equal(string, "1234"));
+  g_assert (g_strlcat(string, "5", 5) == 5);
+  g_assert ( *(string + 4) == '\0' );
+  g_assert (g_str_equal(string, "1234"));
+  g_assert ( *(string + 5) == 'Z' );
+  *string = 'Y';
+  *(string + 1)= '\0';
+  g_assert (g_strlcat(string, "123" , 0) == 3);
+  g_assert (*string == 'Y');
+
+  /* A few more tests, demonstrating more "normal" use  */
+  g_assert (g_strlcpy(string, "hi", 5) == 2);
+  g_assert (g_str_equal(string, "hi"));
+  g_assert (g_strlcat(string, "t", 5) == 3);
+  g_assert (g_str_equal(string, "hit"));
+  g_free(string);
+
+  g_print ("ok\n");
+

   g_print ("checking g_strdup_printf...");
   string = g_strdup_printf ("%05d %-5s", 21, "test");

--- glib.orig/configure.in	Wed Jun 14 01:26:37 2000
+++ glib/configure.in	Wed Jun 14 00:05:29 2000
@@ -1079,6 +1079,22 @@
 	glibconfig-sysdefs.h,
 	=)

+dnl ****************************************
+dnl *** strlcpy/strlcat                  ***
+dnl ****************************************
+# Check for strlcpy
+AC_MSG_CHECKING(for strlcpy/strlcat)
+AC_TRY_LINK([#include <stdlib.h>
+#include <string.h>], [
+char *p = malloc(10);
+(void) strlcpy(p, "hi", 10);
+(void) strlcat(p, "bye", 10);
+], glib_ok=yes, glib_ok=no)
+AC_MSG_RESULT($glib_ok)
+if test $glib_ok = yes; then
+    AC_DEFINE(HAVE_STRLCPY)
+fi
+

 dnl ******************************
 dnl *** output the whole stuff ***

--- glib.orig/acconfig.h	Wed Jun 14 00:02:09 2000
+++ glib/acconfig.h	Wed Jun 14 00:05:29 2000
@@ -65,6 +65,7 @@
 #undef HAVE_PTHREAD_ATTR_SETSTACKSIZE
 #undef HAVE_PWD_H
 #undef HAVE_PW_GECOS
+#undef HAVE_STRLCPY
 #undef HAVE_SYS_PARAM_H
 #undef HAVE_SYS_POLL_H
 #undef HAVE_SYS_SELECT_H



-- 

--- David A. Wheeler
    dwheeler@ida.org