From: bones_was_here <bones_was_here@xonotic.au>
Date: Mon, 18 Dec 2023 09:24:15 +0000 (+1000)
Subject: Fix uses of strlcpy on unterminated source strings
X-Git-Url: https://git.rm.cloudns.org/?a=commitdiff_plain;h=3727057b879ccfeaa434537f41a302f047e8cfae;p=xonotic%2Fdarkplaces.git

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 <bones_was_here@xonotic.au>
---

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++ ) {