From: bones_was_here Date: Wed, 13 Dec 2023 07:16:24 +0000 (+1000) Subject: com: replace BSD strlcpy and strlcat X-Git-Url: http://git.xonotic.org/?p=xonotic%2Fdarkplaces.git;a=commitdiff_plain;h=b60133483aa561097865246a9f94b1e51798ce10 com: replace BSD strlcpy and strlcat Compared to BSD strlcpy and strlcat, dp_strlcpy and dp_strlcat are faster, never crash, and have a more useful return (DP didn't use the strlcpy return at all), see included docs. Adds dp_stpecpy() for efficient chain copying and dp_ustr2stp() for copying measured byte sequences (unterminated strings) to strings. Replaces the only use of the strlcat() return with dp_stpecpy(). Updates the forbidden string functions list. Signed-off-by: bones_was_here --- diff --git a/common.c b/common.c index 0f7ddd2b..05bbd7c6 100644 --- a/common.c +++ b/common.c @@ -1305,80 +1305,87 @@ COM_StringDecolorize(const char *in, size_t size_in, char *out, size_t size_out, #undef APPEND } -//======================================================== -// strlcat and strlcpy, from OpenBSD /* - * Copyright (c) 1998, 2015 Todd C. Miller - * - * Permission to use, copy, modify, and distribute this software for any - * purpose with or without fee is hereby granted, provided that the above - * copyright notice and this permission notice appear in all copies. - * - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +============ +String Copying + +The glibc implementation of memccpy is several times faster than old functions that +copy one byte at a time (even at -O3) and its advantage increases with string length. +============ +*/ +#ifdef WIN32 + // memccpy() is standard in POSIX.1-2001, POSIX.1-2008, SVr4, 4.3BSD, C23. + // Microsoft supports it, but apparently complains if we use it. + #pragma warning(disable : 4996) +#endif + +/** Chain-copies a string with truncation and efficiency (compared to strlcat()). + * The destination ends at an absolute pointer instead of a relative offset + * and a pointer to the \0 terminator is returned on success. + * Truncates, warns, and returns the end pointer on overflow or unterminated source. + * Guarantees \0 termination. end = dst + sizeof(src[]) */ +char *dp_stpecpy(char *dst, char *end, const char *src) +{ + char *p = (char *)memccpy(dst, src, '\0', end - dst); -/* $OpenBSD: strlcat.c,v 1.19 2019/01/25 00:19:25 millert Exp $ */ -/* $OpenBSD: strlcpy.c,v 1.16 2019/01/25 00:19:25 millert Exp $ */ + if (p) + return p - 1; + end[-1] = '\0'; + Con_Printf(CON_WARN "%s: src string unterminated or truncated to %zu bytes: \"%s\"\n", __func__, dst == end ? 0 : (end - dst) - 1, dst); + return end; +} -size_t dp_strlcat(char *dst, const char *src, size_t dsize) +/** Copies a measured byte sequence (unterminated string) to a null-terminated string. + * Returns a pointer to the \0 terminator. Guarantees \0 termination. + * Compared to ustr2stp(): truncates and warns on overflow. + */ +char *dp_ustr2stp(char *dst, size_t dsize, const char *src, size_t ssize) { - const char *odst = dst; - const char *osrc = src; - size_t n = dsize; - size_t dlen; - - /* Find the end of dst and adjust bytes left but don't go past end. */ - while (n-- != 0 && *dst != '\0') - dst++; - dlen = dst - odst; - n = dsize - dlen; - - if (n-- == 0) - return(dlen + strlen(src)); - while (*src != '\0') { - if (n != 0) { - *dst++ = *src; - n--; - } - src++; + if (ssize >= dsize) + { + ssize = dsize - 1; + Con_Printf(CON_WARN "%s: src string truncated to %zu bytes: \"%.*s\"\n", __func__, ssize, (int)ssize, src); } - *dst = '\0'; - - return(dlen + (src - osrc)); /* count does not include NUL */ + memcpy(dst, src, ssize); + dst[ssize] = '\0'; + return &dst[ssize]; } -size_t dp_strlcpy(char *dst, const char *src, size_t dsize) +/** Copies a string, like strlcpy() but with a better return: the number of bytes copied + * excluding the \0 terminator. Truncates and warns on overflow or unterminated source, + * whereas strlcpy() truncates silently and overreads (possibly segfaulting). + * Guarantees \0 termination. + * See also: dp_stpecpy() and dp_ustr2stp(). + */ +size_t dp__strlcpy(char *dst, const char *src, size_t dsize, const char *func, unsigned line) { - const char *osrc = src; - size_t nleft = dsize; + char *p = (char *)memccpy(dst, src, '\0', dsize); - /* Copy as many bytes as will fit. */ - if (nleft != 0) { - while (--nleft != 0) { - if ((*dst++ = *src++) == '\0') - break; - } - } + if (p) + return (p - 1) - dst; + dst[dsize - 1] = '\0'; + Con_Printf(CON_WARN "%s:%u: src string unterminated or truncated to %zu bytes: \"%s\"\n", func, line, dsize - 1, dst); + return dsize - 1; +} - /* Not enough room in dst, add NUL and traverse rest of src. */ - if (nleft == 0) { - if (dsize != 0) - *dst = '\0'; /* NUL-terminate dst */ - while (*src++) - ; - } +/** Catenates a string, like strlcat() but with a better return: the number of bytes copied + * excluding the \0 terminator. Truncates and warns on overflow or unterminated source, + * whereas strlcat() truncates silently and overreads (possibly segfaulting). + * Guarantees \0 termination. + * Inefficient like any strcat(), please use memcpy(), dp_stpecpy() or dp_strlcpy() instead. + */ +size_t dp__strlcat(char *dst, const char *src, size_t dsize, const char *func, unsigned line) +{ + char *p = (char *)memchr(dst, '\0', dsize) ?: dst; + size_t offset = p - dst; - return(src - osrc - 1); /* count does not include NUL */ + return dp__strlcpy(p, src, dsize - offset, func, line) + offset; } + void FindFraction(double val, int *num, int *denom, int denomMax) { int i; diff --git a/common.h b/common.h index 677219d6..ff088a79 100644 --- a/common.h +++ b/common.h @@ -225,13 +225,9 @@ char *va(char *buf, size_t buflen, const char *format, ...) DP_FUNC_PRINTF(3); #endif // snprintf and vsnprintf are NOT portable. Use their DP counterparts instead -#ifdef snprintf -# undef snprintf -#endif +#undef snprintf #define snprintf DP_STATIC_ASSERT(0, "snprintf is forbidden for portability reasons. Use dpsnprintf instead.") -#ifdef vsnprintf -# undef vsnprintf -#endif +#undef vsnprintf #define vsnprintf DP_STATIC_ASSERT(0, "vsnprintf is forbidden for portability reasons. Use dpvsnprintf instead.") // dpsnprintf and dpvsnprintf @@ -244,16 +240,27 @@ extern int dpvsnprintf (char *buffer, size_t buffersize, const char *format, va_ // A bunch of functions are forbidden for security reasons (and also to please MSVS 2005, for some of them) // LadyHavoc: added #undef lines here to avoid warnings in Linux #undef strcat -#define strcat DP_STATIC_ASSERT(0, "strcat is forbidden for security reasons. Use strlcat or memcpy instead.") +#define strcat DP_STATIC_ASSERT(0, "strcat is forbidden for security reasons. Use dp_strlcat or memcpy instead.") #undef strncat -#define strncat DP_STATIC_ASSERT(0, "strncat is forbidden for security reasons. Use strlcat or memcpy instead.") +#define strncat DP_STATIC_ASSERT(0, "strncat is forbidden for security reasons. Use dp_strlcat or memcpy instead.") #undef strcpy -#define strcpy DP_STATIC_ASSERT(0, "strcpy is forbidden for security reasons. Use strlcpy or memcpy instead.") +#define strcpy DP_STATIC_ASSERT(0, "strcpy is forbidden for security reasons. Use dp_strlcpy or memcpy instead.") #undef strncpy -#define strncpy DP_STATIC_ASSERT(0, "strncpy is forbidden for security reasons. Use strlcpy or memcpy instead.") +#define strncpy DP_STATIC_ASSERT(0, "strncpy is forbidden for security reasons. Use dp_strlcpy or memcpy instead.") +#undef stpcpy +#define stpcpy DP_STATIC_ASSERT(0, "stpcpy is forbidden for security reasons. Use dp_stpecpy or memcpy instead.") +#undef ustpcpy +#define ustpcpy DP_STATIC_ASSERT(0, "ustpcpy is forbidden for security reasons. Use dp_ustr2stp or memcpy instead.") +#undef ustr2stp +#define ustr2stp DP_STATIC_ASSERT(0, "ustr2stp is forbidden for security reasons. Use dp_ustr2stp or memcpy instead.") #undef sprintf #define sprintf DP_STATIC_ASSERT(0, "sprintf is forbidden for security reasons. Use dpsnprintf instead.") +#undef strlcpy +#define strlcpy DP_STATIC_ASSERT(0, "strlcpy is forbidden for stability and correctness. See common.h and common.c comments.") +#undef strlcat +#define strlcat DP_STATIC_ASSERT(0, "strlcat is forbidden for stability and correctness. See common.h and common.c comments.") + //============================================================================ @@ -281,21 +288,14 @@ qbool COM_StringDecolorize(const char *in, size_t size_in, char *out, size_t siz void COM_ToLowerString (const char *in, char *out, size_t size_out); void COM_ToUpperString (const char *in, char *out, size_t size_out); -/*! - * Appends src to string dst of size dsize (unlike strncat, dsize is the - * full size of dst, not space left). At most dsize-1 characters - * will be copied. Always NUL terminates (unless dsize <= strlen(dst)). - * Returns strlen(src) + MIN(dsize, strlen(initial dst)). - * If retval >= dsize, truncation occurred. - */ -size_t dp_strlcat(char *dst, const char *src, size_t dsize); -/*! - * Copy string src to buffer dst of size dsize. At most dsize-1 - * chars will be copied. Always NUL terminates (unless dsize == 0). - * Returns strlen(src); if retval >= dsize, truncation occurred. - */ -size_t dp_strlcpy(char *dst, const char *src, size_t dsize); +#define dp_strlcpy(dst, src, dsize) dp__strlcpy(dst, src, dsize, __func__, __LINE__) +#define dp_strlcat(dst, src, dsize) dp__strlcat(dst, src, dsize, __func__, __LINE__) +size_t dp__strlcpy(char *dst, const char *src, size_t dsize, const char *func, unsigned line); +size_t dp__strlcat(char *dst, const char *src, size_t dsize, const char *func, unsigned line); +char *dp_stpecpy(char *dst, char *end, const char *src); +char *dp_ustr2stp(char *dst, size_t dsize, const char *src, size_t ssize); + void FindFraction(double val, int *num, int *denom, int denomMax); diff --git a/prvm_exec.c b/prvm_exec.c index e3ad8a82..ee738b9c 100644 --- a/prvm_exec.c +++ b/prvm_exec.c @@ -434,10 +434,11 @@ void PRVM_ShortStackTrace(prvm_prog_t *prog, char *buf, size_t bufsize) mfunction_t *f; int i; char vabuf[1024]; + char *p; if(prog) { - dpsnprintf(buf, bufsize, "(%s) ", prog->name); + p = buf + max(0, dpsnprintf(buf, bufsize, "(%s) ", prog->name)); } else { @@ -450,13 +451,14 @@ void PRVM_ShortStackTrace(prvm_prog_t *prog, char *buf, size_t bufsize) for (i = prog->depth;i > 0;i--) { f = prog->stack[i].f; - - if(dp_strlcat(buf, + p = dp_stpecpy( + p, + buf + bufsize, f ? va(vabuf, sizeof(vabuf), "%s:%s(%i) ", PRVM_GetString(prog, f->s_file), PRVM_GetString(prog, f->s_name), prog->stack[i].s - f->first_statement) - : " ", - bufsize - ) >= bufsize) + : " " + ); + if (p == buf + bufsize) break; } }