From 3727057b879ccfeaa434537f41a302f047e8cfae Mon Sep 17 00:00:00 2001 From: bones_was_here Date: Mon, 18 Dec 2023 19:24:15 +1000 Subject: [PATCH] Fix uses of strlcpy on unterminated source strings BSD strlcpy() keeps reading until a \0 or a segfault, even after it stops writing. In these cases the source was part of a longer string so it wouldn't have crashed. These were caught by the new warning. The +1 on the source sizes copied an extra char that would be changed to \0 and is now redundant because the real dest and src sizes are always passed to a purpose-written func. Signed-off-by: bones_was_here --- cmd.c | 5 +++-- cmd.h | 4 ++-- console.c | 7 ++++--- fs.c | 4 ++-- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/cmd.c b/cmd.c index 8c6570ec..667b9009 100644 --- a/cmd.c +++ b/cmd.c @@ -169,7 +169,8 @@ static cmd_input_t *Cbuf_NodeGet(cmd_buf_t *cbuf, cmd_input_t *existing) ============ Cbuf_LinkString -Copies a command string into a buffer node +Copies a command string into a buffer node. +The input should not be null-terminated, the output will be. ============ */ static void Cbuf_LinkString(cmd_state_t *cmd, llist_t *head, cmd_input_t *existing, const char *text, qbool leavepending, unsigned int cmdsize) @@ -193,7 +194,7 @@ static void Cbuf_LinkString(cmd_state_t *cmd, llist_t *head, cmd_input_t *existi } cbuf->size += cmdsize; - dp_strlcpy(&node->text[offset], text, cmdsize + 1); // always sets the last char to \0 + dp_ustr2stp(&node->text[offset], node->length + 1, text, cmdsize); //Con_Printf("^5Cbuf_LinkString(): %s `^7%s^5`\n", node->pending ? "append" : "new", &node->text[offset]); node->pending = leavepending; } diff --git a/cmd.h b/cmd.h index 30b3abd0..614aa0a0 100644 --- a/cmd.h +++ b/cmd.h @@ -156,8 +156,8 @@ typedef struct cmd_input_s llist_t list; cmd_state_t *source; vec_t delay; - size_t size; - size_t length; + size_t size; ///< excludes \0 terminator + size_t length; ///< excludes \0 terminator char *text; qbool pending; } cmd_input_t; diff --git a/console.c b/console.c index 7767617c..eefcb25e 100644 --- a/console.c +++ b/console.c @@ -391,8 +391,8 @@ const char *ConBuffer_GetLine(conbuffer_t *buf, int i) { static char copybuf[MAX_INPUTLINE]; // client only con_lineinfo_t *l = &CONBUFFER_LINES(buf, i); - size_t sz = l->len+1 > sizeof(copybuf) ? sizeof(copybuf) : l->len+1; - dp_strlcpy(copybuf, l->start, sz); + + dp_ustr2stp(copybuf, sizeof(copybuf), l->start, l->len); return copybuf; } @@ -2785,7 +2785,8 @@ int Con_CompleteCommandLine(cmd_state_t *cmd, qbool is_console) space = strchr(line + 1, ' '); if(space && pos == (space - line) + 1) { - dp_strlcpy(command, line + 1, min(sizeof(command), (unsigned int)(space - line))); + // adding 1 to line drops the leading ] + dp_ustr2stp(command, sizeof(command), line + 1, space - (line + 1)); patterns = Cvar_VariableString(cmd->cvars, va(vabuf, sizeof(vabuf), "con_completion_%s", command), CF_CLIENT | CF_SERVER); // TODO maybe use a better place for this? if(patterns && !*patterns) diff --git a/fs.c b/fs.c index 9469956a..c76a1467 100644 --- a/fs.c +++ b/fs.c @@ -3846,7 +3846,7 @@ fssearch_t *FS_Search(const char *pattern, int caseinsensitive, int quiet, const // prevseparator points past the '/' right before the wildcard and nextseparator at the one following it (or at the end of the string) // copy everything up except nextseperator - dp_strlcpy(subpattern, pattern, min(sizeof(subpattern), (size_t) (nextseparator - pattern + 1))); + dp_ustr2stp(subpattern, sizeof(subpattern), pattern, nextseparator - pattern); // find the last '/' before the wildcard prevseparator = strrchr( subpattern, '/' ); if (!prevseparator) @@ -3855,7 +3855,7 @@ fssearch_t *FS_Search(const char *pattern, int caseinsensitive, int quiet, const prevseparator++; // copy everything from start to the previous including the '/' (before the wildcard) // everything up to start is already included in the path of matchedSet's entries - dp_strlcpy(subpath, start, min(sizeof(subpath), (size_t) ((prevseparator - subpattern) - (start - pattern) + 1))); + dp_ustr2stp(subpath, sizeof(subpath), start, (prevseparator - subpattern) - (start - pattern)); // for each entry in matchedSet try to open the subdirectories specified in subpath for( dirlistindex = 0 ; dirlistindex < matchedSet.numstrings ; dirlistindex++ ) { -- 2.39.2