Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 14 Jun 2023 13:26:32 GMT
From:      =?utf-8?Q?Dag-Erling=20Sm=C3=B8rgrav?= <des@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: bc47005a648b - stable/13 - xargs: terminate if line replacement cannot be constructed
Message-ID:  <202306141326.35EDQWrO099524@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=bc47005a648b1b326cfd852c039a9b01854c5796

commit bc47005a648b1b326cfd852c039a9b01854c5796
Author:     Tom Jones <thj@FreeBSD.org>
AuthorDate: 2022-07-05 15:03:51 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2023-06-14 12:48:55 +0000

    xargs: terminate if line replacement cannot be constructed
    
    If the line with replacement cannot be constructed xargs should
    terminate as documented in the man page
    
    We encounter this error, but gnu/xargs doesn't because they have a much
    larger limit for created outputs (~10000 lines).
    
    Reviewed by:    oshogbo
    Sponsored by:   Klara, Inc.
    Differential Revision:  https://reviews.freebsd.org/D35574
    
    (cherry picked from commit f058359ba5e08c555d7e6f192217f890b83cd46c)
    
    xargs: fix description of strnsubst return value
    
    Reported by:    oshogbo
    Sponsored by:   Klara, Inc.
    Differential Revision:  https://reviews.freebsd.org/D35574
    
    (cherry picked from commit 1e692b938e37a9b43a43ace2739eb6b97379cac0)
    
    xargs: improve foundeof check for -E
    
    4aeb63826e0f got it almost correct (we can't use strcmp() here as
    current argument isn't guaranteed to be NUL-terminated), but we also
    need to check that current argument length is equal to that of eofstr.
    
    PR:             270867
    Reviewed by:    bapt
    Differential Revision:  https://reviews.freebsd.org/D39583
    
    (cherry picked from commit 21ef48af5c0f4ed85a5f42855b5a8d58b5431103)
    
    xargs: Fix typo in error message.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    
    (cherry picked from commit 6d777389e19d3bebde515e88e8405de45d8af7bd)
    
    xargs: Consistently use strtonum() to parse arguments.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    kevans
    Differential Revision:  https://reviews.freebsd.org/D40425
    
    (cherry picked from commit fbc445addf9183d180bb8b488281617bb19d9242)
---
 usr.bin/xargs/strnsubst.c | 15 ++++++++++-----
 usr.bin/xargs/xargs.c     | 44 +++++++++++++++++++++++++-------------------
 2 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/usr.bin/xargs/strnsubst.c b/usr.bin/xargs/strnsubst.c
index d826d33c0967..4a2ecd5aa9a8 100644
--- a/usr.bin/xargs/strnsubst.c
+++ b/usr.bin/xargs/strnsubst.c
@@ -12,11 +12,12 @@
 __FBSDID("$FreeBSD$");
 
 #include <err.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
-void	strnsubst(char **, const char *, const char *, size_t);
+bool	strnsubst(char **, const char *, const char *, size_t);
 
 /*
  * Replaces str with a string consisting of str with match replaced with
@@ -26,16 +27,19 @@ void	strnsubst(char **, const char *, const char *, size_t);
  * str as well as the new contents are handled in an appropriate manner.
  * If replstr is NULL, then that internally is changed to a nil-string, so
  * that we can still pretend to do somewhat meaningful substitution.
- * No value is returned.
+ *
+ * Returns true if truncation was needed to do the replacement, false if
+ * truncation was not required.
  */
-void
+bool
 strnsubst(char **str, const char *match, const char *replstr, size_t maxsize)
 {
 	char *s1, *s2, *this;
+	bool error = false;
 
 	s1 = *str;
 	if (s1 == NULL)
-		return;
+		return false;
 	/*
 	 * If maxsize is 0 then set it to the length of s1, because we have
 	 * to duplicate s1.  XXX we maybe should double-check whether the match
@@ -67,6 +71,7 @@ strnsubst(char **str, const char *match, const char *replstr, size_t maxsize)
 		if ((strlen(s2) + strlen(s1) + strlen(replstr) -
 		    strlen(match) + 1) > maxsize) {
 			strlcat(s2, s1, maxsize);
+			error = true;
 			goto done;
 		}
 		strncat(s2, s1, (uintptr_t)this - (uintptr_t)s1);
@@ -76,7 +81,7 @@ strnsubst(char **str, const char *match, const char *replstr, size_t maxsize)
 	strcat(s2, s1);
 done:
 	*str = s2;
-	return;
+	return error;
 }
 
 #ifdef TEST
diff --git a/usr.bin/xargs/xargs.c b/usr.bin/xargs/xargs.c
index 6520d43dae50..e832cbb35a31 100644
--- a/usr.bin/xargs/xargs.c
+++ b/usr.bin/xargs/xargs.c
@@ -61,6 +61,7 @@ __FBSDID("$FreeBSD$");
 #include <locale.h>
 #include <paths.h>
 #include <regex.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -73,7 +74,7 @@ static void	prerun(int, char *[]);
 static int	prompt(void);
 static void	run(char **);
 static void	usage(void);
-void		strnsubst(char **, const char *, const char *, size_t);
+bool		strnsubst(char **, const char *, const char *, size_t);
 static pid_t	xwait(int block, int *status);
 static void	xexit(const char *, const int);
 static void	waitchildren(const char *, int);
@@ -90,6 +91,7 @@ static char echo[] = _PATH_ECHO;
 static char **av, **bxp, **ep, **endxp, **xp;
 static char *argp, *bbp, *ebp, *inpline, *p, *replstr;
 static const char *eofstr;
+static long eoflen;
 static int count, insingle, indouble, oflag, pflag, tflag, Rflag, rval, zflag;
 static int cnt, Iflag, jfound, Lflag, Sflag, wasquoted, xflag;
 static int curprocs, maxprocs;
@@ -122,12 +124,12 @@ main(int argc, char *argv[])
 	int ch, Jflag, nargs, nflag, nline;
 	size_t linelen;
 	struct rlimit rl;
-	char *endptr;
 	const char *errstr;
 
 	inpline = replstr = NULL;
 	ep = environ;
 	eofstr = "";
+	eoflen = 0;
 	Jflag = nflag = 0;
 
 	(void)setlocale(LC_ALL, "");
@@ -158,6 +160,7 @@ main(int argc, char *argv[])
 		switch (ch) {
 		case 'E':
 			eofstr = optarg;
+			eoflen = strlen(eofstr);
 			break;
 		case 'I':
 			Jflag = 0;
@@ -171,23 +174,23 @@ main(int argc, char *argv[])
 			replstr = optarg;
 			break;
 		case 'L':
-			Lflag = strtonum(optarg, 0, INT_MAX, &errstr);
+			Lflag = (int)strtonum(optarg, 0, INT_MAX, &errstr);
 			if (errstr)
-				errx(1, "-L %s: %s", optarg, errstr);
+				errx(1, "-%c %s: %s", ch, optarg, errstr);
 			break;
 		case 'n':
 			nflag = 1;
-			nargs = strtonum(optarg, 1, INT_MAX, &errstr);
+			nargs = (int)strtonum(optarg, 1, INT_MAX, &errstr);
 			if (errstr)
-				errx(1, "-n %s: %s", optarg, errstr);
+				errx(1, "-%c %s: %s", ch, optarg, errstr);
 			break;
 		case 'o':
 			oflag = 1;
 			break;
 		case 'P':
-			maxprocs = strtonum(optarg, 0, INT_MAX, &errstr);
+			maxprocs = (int)strtonum(optarg, 0, INT_MAX, &errstr);
 			if (errstr)
-				errx(1, "-P %s: %s", optarg, errstr);
+				errx(1, "-%c %s: %s", ch, optarg, errstr);
 			if (getrlimit(RLIMIT_NPROC, &rl) != 0)
 				errx(1, "getrlimit failed");
 			if (maxprocs == 0 || maxprocs > rl.rlim_cur)
@@ -197,22 +200,22 @@ main(int argc, char *argv[])
 			pflag = 1;
 			break;
 		case 'R':
-			Rflag = strtol(optarg, &endptr, 10);
-			if (*endptr != '\0')
-				errx(1, "replacements must be a number");
+			Rflag = (int)strtonum(optarg, 0, INT_MAX, &errstr);
+			if (errstr)
+				errx(1, "-%c %s: %s", ch, optarg, errstr);
 			break;
 		case 'r':
 			/* GNU compatibility */
 			break;
 		case 'S':
-			Sflag = strtoul(optarg, &endptr, 10);
-			if (*endptr != '\0')
-				errx(1, "replsize must be a number");
+			Sflag = (int)strtonum(optarg, 0, INT_MAX, &errstr);
+			if (errstr)
+				errx(1, "-%c %s: %s", ch, optarg, errstr);
 			break;
 		case 's':
-			nline = strtonum(optarg, 0, INT_MAX, &errstr);
+			nline = (int)strtonum(optarg, 0, INT_MAX, &errstr);
 			if (errstr)
-				errx(1, "-s %s: %s", optarg, errstr);
+				errx(1, "-%c %s: %s", ch, optarg, errstr);
 			break;
 		case 't':
 			tflag = 1;
@@ -347,8 +350,8 @@ arg1:		if (insingle || indouble) {
 			xexit(*av, 1);
 		}
 arg2:
-		foundeof = *eofstr != '\0' &&
-		    strncmp(argp, eofstr, p - argp) == 0;
+		foundeof = eoflen != 0 && p - argp == eoflen &&
+		    strncmp(argp, eofstr, eoflen) == 0;
 
 		/* Do not make empty args unless they are quoted */
 		if ((argp != p || wasquoted) && !foundeof) {
@@ -521,7 +524,10 @@ prerun(int argc, char *argv[])
 	while (--argc) {
 		*tmp = *avj++;
 		if (repls && strstr(*tmp, replstr) != NULL) {
-			strnsubst(tmp++, replstr, inpline, (size_t)Sflag);
+			if (strnsubst(tmp++, replstr, inpline, (size_t)Sflag)) {
+				warnx("command line cannot be assembled, too long");
+				xexit(*argv, 1);
+			}
 			if (repls > 0)
 				repls--;
 		} else {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202306141326.35EDQWrO099524>