library/1884: strlcat() should always NUL terminate dst, maybe

[gphilip2@qwest.net]

>Number:         1884
>Category:       library
>Synopsis:       strlcat() should always NUL terminate dst, maybe
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bugs
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sat Jun 16 21:30:02 MDT 2001
>Last-Modified:
>Originator:     Philip Guenther
>Organization:
net
>Release:        current
>Environment:
	
	System      : OpenBSD 2.9
	Architecture: OpenBSD.i386
	Machine     : i386
>Description:
	As others have pointed out, if strlcat() is passed a dst
	that isn't NUL terminated within the first siz bytes, then
	something has obviously gone wrong already.

	When I read the manpage to see what strlcat() does in this
	case, it initially interpreted it as saying that strlcat()
	and strlcpy() would *always* return with dst NUL terminated.
	A look at the source showed otherwise, of course.

	I would argue that it is more useful for strlcat() to provide
	the invariant "siz == 0 || strlen(dst) < siz" than to leave
	the string untouched in that case.

	In a sense, I would take the decision from the 1.6-->1.7
	patch on strlcat.c one step further: not only will strlcat()
	not read or write beyond dst[siz-1], but it'll try to prevent
	others from erroneously doing so.

>How-To-Repeat:

	morgaine% cat foo.c
	#include <stdio.h>
	#include <string.h>
	int main(void)
	{ char buf[] = "foobar";
	  size_t len;

	  len = strlcat(buf,"baz",3);
	  printf("buf = '%s'\nlen = %u\n", buf, len);
	  return 0;
	}
	morgaine% cc foo.c && ./a.out
	buf = 'foobar'
	len = 6
	morgaine% 

>Fix:
	

	This patch is against the main branch, not branch OPENBSD_2_9.

Index: strlcat.c
===================================================================
RCS file: /home/cvs/openbsd/src/lib/libc/string/strlcat.c,v
retrieving revision 1.8
diff -c -c -r1.8 strlcat.c
*** strlcat.c	2001/05/13 15:40:15	1.8
--- strlcat.c	2001/06/17 03:02:07
***************
*** 58,65 ****
  	dlen = d - dst;
  	n = siz - dlen;
  
! 	if (n == 0)
  		return(dlen + strlen(s));
  	while (*s != '\0') {
  		if (n != 1) {
  			*d++ = *s;
--- 58,68 ----
  	dlen = d - dst;
  	n = siz - dlen;
  
! 	if (n == 0) {
! 		if (siz > 0)
! 			d[-1] = '\0';
  		return(dlen + strlen(s));
+ 	}
  	while (*s != '\0') {
  		if (n != 1) {
  			*d++ = *s;
Index: strlcpy.3
===================================================================
RCS file: /home/cvs/openbsd/src/lib/libc/string/strlcpy.3,v
retrieving revision 1.12
diff -c -c -r1.12 strlcpy.3
*** strlcpy.3	2001/05/07 15:51:54	1.12
--- strlcpy.3	2001/06/17 02:55:25
***************
*** 135,141 ****
  .Fa dst
  is not a proper
  .Dq C
! string).
  The check exists to prevent potential security problems in incorrect code.
  .Sh EXAMPLES
  The following code fragment illustrates the simple case:
--- 135,146 ----
  .Fa dst
  is not a proper
  .Dq C
! string).  As long as
! .Fa size
! is not zero,
! .Fn strlcat
! will still NUL terminate
! .Fa dst .
  The check exists to prevent potential security problems in incorrect code.
  .Sh EXAMPLES
  The following code fragment illustrates the simple case:

>Audit-Trail:
>Unformatted: