From 5d354709047d762de49ee774c57d23b877c33736 Mon Sep 17 00:00:00 2001 From: deraadt Date: Sat, 19 Apr 2014 11:30:40 +0000 Subject: [PATCH] Use somewhat harsher language and better examples; demonstrate that non-dangerous use functions is difficult. ok guenther --- lib/libc/string/strcat.3 | 21 +++++----- lib/libc/string/strcpy.3 | 34 ++++++---------- lib/libc/string/strncat.3 | 85 +++++++++++++++++++++------------------ lib/libc/string/strncpy.3 | 57 ++++++++++---------------- 4 files changed, 88 insertions(+), 109 deletions(-) diff --git a/lib/libc/string/strcat.3 b/lib/libc/string/strcat.3 index fba992edd9b..7368d08e4b3 100644 --- a/lib/libc/string/strcat.3 +++ b/lib/libc/string/strcat.3 @@ -1,4 +1,4 @@ -.\" $OpenBSD: strcat.3,v 1.16 2013/12/19 20:52:37 millert Exp $ +.\" $OpenBSD: strcat.3,v 1.17 2014/04/19 11:30:40 deraadt Exp $ .\" .\" Copyright (c) 1990, 1991 The Regents of the University of California. .\" All rights reserved. @@ -31,12 +31,12 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd $Mdocdate: December 19 2013 $ +.Dd $Mdocdate: April 19 2014 $ .Dt STRCAT 3 .Os .Sh NAME .Nm strcat -.Nd concatenate two strings +.Nd concatenate two strings without bounds checking .Sh SYNOPSIS .In string.h .Ft char * @@ -50,22 +50,19 @@ to the end of the NUL-terminated string .Fa s , then adds a terminating .Ql \e0 . -The string -.Fa s -must have sufficient space to hold the result. +.Pp +No bounds checking is performed. +If the buffer +.Fa dst +is not large enough to hold the result, +subsequent memory will be damaged. .Sh RETURN VALUES The .Fn strcat function return the pointer .Fa s . .Sh SEE ALSO -.Xr bcopy 3 , -.Xr memccpy 3 , -.Xr memcpy 3 , -.Xr memmove 3 , -.Xr strcpy 3 , .Xr strlcpy 3 , -.Xr strncat 3 , .Xr wcscat 3 , .Xr wcslcpy 3 .Sh STANDARDS diff --git a/lib/libc/string/strcpy.3 b/lib/libc/string/strcpy.3 index 849184d1f54..7174f7c963c 100644 --- a/lib/libc/string/strcpy.3 +++ b/lib/libc/string/strcpy.3 @@ -1,4 +1,4 @@ -.\" $OpenBSD: strcpy.3,v 1.20 2013/12/19 20:52:37 millert Exp $ +.\" $OpenBSD: strcpy.3,v 1.21 2014/04/19 11:30:40 deraadt Exp $ .\" .\" Copyright (c) 1990, 1991 The Regents of the University of California. .\" All rights reserved. @@ -31,12 +31,12 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd $Mdocdate: December 19 2013 $ +.Dd $Mdocdate: April 19 2014 $ .Dt STRCPY 3 .Os .Sh NAME .Nm strcpy -.Nd copy a string +.Nd copy a string without bounds checking .Sh SYNOPSIS .In string.h .Ft char * @@ -46,43 +46,35 @@ The .Fn strcpy function copies the string .Fa src -to -.Fa dst (including the terminating .Ql \e0 -character). -The string +character) to the buffer +.Fa dst . +.Pp +No bounds checking is performed. +If the buffer .Fa dst -must be at least as large as -.Fa src -to hold the result. +is not large enough to hold the result, +subsequent memory will be damaged. .Pp If the .Fa src -and +string is inside the .Fa dst -strings overlap, the behavior is undefined. +buffer, the behavior is undefined. .Sh RETURN VALUES The .Fn strcpy function returns .Fa dst . .Sh SEE ALSO -.Xr bcopy 3 , -.Xr memccpy 3 , -.Xr memcpy 3 , -.Xr memmove 3 , -.Xr strcat 3 , .Xr strlcpy 3 , -.Xr strncpy 3 , .Xr wcscpy 3 , .Xr wcslcpy 3 .Sh STANDARDS The .Fn strcpy -and -.Fn strncpy -functions conform to +function conforms to .St -ansiC . .Sh HISTORY The diff --git a/lib/libc/string/strncat.3 b/lib/libc/string/strncat.3 index bd15ef10fa2..c0a0da57c71 100644 --- a/lib/libc/string/strncat.3 +++ b/lib/libc/string/strncat.3 @@ -1,4 +1,4 @@ -.\" $OpenBSD: strncat.3,v 1.2 2013/12/19 22:00:58 jmc Exp $ +.\" $OpenBSD: strncat.3,v 1.3 2014/04/19 11:30:40 deraadt Exp $ .\" .\" Copyright (c) 1990, 1991 The Regents of the University of California. .\" All rights reserved. @@ -31,7 +31,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd $Mdocdate: December 19 2013 $ +.Dd $Mdocdate: April 19 2014 $ .Dt STRNCAT 3 .Os .Sh NAME @@ -40,86 +40,91 @@ .Sh SYNOPSIS .In string.h .Ft char * -.Fn strncat "char *s" "const char *append" "size_t count" +.Fn strncat "char *dst" "const char *append" "size_t count" .Sh DESCRIPTION The .Fn strncat function appends not more than .Fa count -characters of the NUL-terminated string +characters of the string .Fa append -to the end of the NUL-terminated string -.Fa s . +to the end of the string found in the buffer +.Fa dst . Space for the terminating .Ql \e0 should not be included in .Fa count . -The string -.Fa s -must have sufficient space to hold the result. +.Pp +Bounds checking must be performed manually with great care. +If the buffer +.Fa dst +is not large enough to hold the result, +subsequent memory will be damaged. .Sh RETURN VALUES The .Fn strncat function returns the pointer -.Fa s . +.Fa dst . .Sh EXAMPLES -The following appends -.Dq Li abc -to -.Va chararray : -.Bd -literal -offset indent -char *letters = "abcdefghi"; - -(void)strncat(chararray, letters, 3); -.Ed -.Pp The following example shows how to use .Fn strncat -safely in conjunction with -.Xr strncpy 3 . +in conjunction with +.Xr strncpy 3 : .Bd -literal -offset indent char buf[BUFSIZ]; -char *input, *suffix; +char *base, *suffix; -(void)strncpy(buf, input, sizeof(buf) - 1); +(void)strncpy(buf, base, sizeof(buf) - 1); buf[sizeof(buf) - 1] = '\e0'; (void)strncat(buf, suffix, sizeof(buf) - 1 - strlen(buf)); .Ed .Pp The above will copy as many characters from -.Va input +.Va base to .Va buf as will fit. It then appends as many characters from .Va suffix -as will fit (or none -if there is no space). -For operations like this, the +as will fit. +If either +.Va base +or +.Va suffix +are too large, truncation will occur without detection. +.Pp +The above example shows dangerous coding patterns, including an +inability to detect truncation. +.Fn strncat +and +.Fn strncpy +are dangerously easy to misuse. +The .Xr strlcpy 3 and .Xr strlcat 3 -functions are a better choice, as shown below. +functions are safer for this kind of operation: +.Bd -literal -offset indent +if (strlcpy(buf, base, sizeof(buf)) >= sizeof(buf) || + strlcat(buf, suffix, sizeof(buf)) >= sizeof(buf)) + goto toolong; + +.Ed +or for greatest portability, .Bd -literal -offset indent -(void)strlcpy(buf, input, sizeof(buf)); -(void)strlcat(buf, suffix, sizeof(buf)); +if (snprintf(buf, sizeof(buf), "%s%s", + base, suffix) >= sizeof(buf)) + goto toolong; .Ed + .Sh SEE ALSO -.Xr bcopy 3 , -.Xr memccpy 3 , -.Xr memcpy 3 , -.Xr memmove 3 , -.Xr strcat 3 , -.Xr strcpy 3 , .Xr strlcpy 3 , .Xr wcscat 3 , .Xr wcslcpy 3 .Sh STANDARDS The -.Fn strcat -and .Fn strncat -functions conform to +function conform to .St -ansiC . .Sh HISTORY The diff --git a/lib/libc/string/strncpy.3 b/lib/libc/string/strncpy.3 index dd8ddb86fca..3a68a0bd5b8 100644 --- a/lib/libc/string/strncpy.3 +++ b/lib/libc/string/strncpy.3 @@ -1,4 +1,4 @@ -.\" $OpenBSD: strncpy.3,v 1.1 2013/12/19 20:52:37 millert Exp $ +.\" $OpenBSD: strncpy.3,v 1.2 2014/04/19 11:30:40 deraadt Exp $ .\" .\" Copyright (c) 1990, 1991 The Regents of the University of California. .\" All rights reserved. @@ -31,7 +31,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd $Mdocdate: December 19 2013 $ +.Dd $Mdocdate: April 19 2014 $ .Dt STRNCPY 3 .Os .Sh NAME @@ -48,17 +48,16 @@ function copies not more than .Fa len characters from the string .Fa src -to +to the buffer .Fa dst . If .Fa src is less than .Fa len characters long, -it appends +it fills the remaining buffer with .Ql \e0 -characters for the rest of -.Fa len . +characters. If the length of .Fa src is greater than or equal to @@ -68,6 +67,11 @@ will .Em not be NUL-terminated. .Pp +.Fn strncpy +.Em only +NUL terminates the destination string when the length of the source +string is less than the length parameter. +.Pp If the .Fa src and @@ -90,31 +94,17 @@ to The following sets .Va chararray to -.Dq abcdef -and does -.Em not -NUL terminate -.Va chararray -because the length of the source string is greater than or equal to the -length parameter. -.Fn strncpy -.Em only -NUL terminates the destination string when the length of the source -string is less than the length parameter. +.Dq abcdef , +without a NUL-terminator: .Bd -literal -offset indent (void)strncpy(chararray, "abcdefgh", 6); .Ed .Pp -The following copies as many characters from +The following sequence copies as many characters from .Va input to .Va buf -as will fit and NUL terminates the result. -Because -.Fn strncpy -does -.Em not -guarantee to NUL terminate the string itself, it must be done by hand. +as will fit, and then NUL terminates the result by hand: .Bd -literal -offset indent char buf[BUFSIZ]; @@ -122,23 +112,18 @@ char buf[BUFSIZ]; buf[sizeof(buf) - 1] = '\e0'; .Ed .Pp -Note that -.Xr strlcpy 3 -is a better choice for this kind of operation. -The equivalent using +By now it is clear that +.Nm strncpy +is dangerously easy to misuse. +The .Xr strlcpy 3 -is simply: +function is safer for this kind of operation: .Bd -literal -offset indent -(void)strlcpy(buf, input, sizeof(buf)); +if (strlcpy(buf, input, sizeof(buf)) >= sizeof(buf)) + goto toolong; .Ed .Sh SEE ALSO -.Xr bcopy 3 , -.Xr memccpy 3 , -.Xr memcpy 3 , -.Xr memmove 3 , -.Xr strcat 3 , .Xr strlcpy 3 , -.Xr strncat 3 , .Xr wcscpy 3 , .Xr wcslcpy 3 .Sh STANDARDS -- 2.20.1