[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1036175: unblock: mksh/59c-28



Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock
X-Debbugs-Cc: mksh@packages.debian.org, tg@mirbsd.de
Control: affects -1 + src:mksh

Please unblock package mksh

[ Reason ]
Some bugfixes regarding formatting, shift/rotate arithmetics,
and a few cases where the parser was not properly reentrant
leading to use-after-free/double-free for recursive parsing.
Also add some tests for the printf builtin to the testsuite.

[ Impact ]
Probably low. I haven’t hit any of these in practice, but
fuzzing would find them.

[ Tests ]
There is a new test for the printf builtin, but the other
changes have only been manually tested to not crash (also
with Valgrind and ASan/UBSan) after the change.

[ Risks ]
I’ve only cherry-picked the changes I’m fully confident of
and that do not touch larger areas of code. They have been
in use for several weeks, and no new issues popped up.

[ Checklist ]
  [x] all changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [↓] attach debdiff against the package in testing

[ Other info ]
Instead of a debdiff, because all changes are contained in
d/patches/debian-changes, I attached a diff of the unpacked
and patched package minus d/patches/.

The changes to Build.sh and check.t are to test the printf builtin.

The change to mbsdint.h is the arithmetic fix: a % b instead of a & (b-1)

The change to shf.c is to fix the formatting stuff.

The changes to sh.h and syn.c are the parser fixes, plus sh.h
assigns a unique upstream revision date to the so-patched version
to ease $KSH_VERSION-based bugspotting.

I cherry-picked the fixes only but kept the RCS IDs as is in order
to not clutter the diff for easier review.


unblock mksh/59c-28
diff -pruN mksh-59c-26/Build.sh mksh-59c-28/Build.sh
--- mksh-59c-26/Build.sh	2023-05-16 19:13:21.000000000 +0200
+++ mksh-59c-28/Build.sh	2023-05-16 19:13:32.000000000 +0200
@@ -2805,6 +2805,7 @@ fi
 
 if test 1 = "$USE_PRINTF_BUILTIN"; then
 	cpp_define MKSH_PRINTF_BUILTIN 1
+	check_categories="$check_categories printf-builtin"
 else
 	USE_PRINTF_BUILTIN=0
 fi
diff -pruN mksh-59c-26/check.t mksh-59c-28/check.t
--- mksh-59c-26/check.t	2023-05-16 19:13:21.000000000 +0200
+++ mksh-59c-28/check.t	2023-05-16 19:13:32.000000000 +0200
@@ -31,7 +31,7 @@
 # (2013/12/02 20:39:44) http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/regress/bin/ksh/?sortby=date
 
 expected-stdout:
-	KSH R59 2023/01/31
+	KSH R59 2023/04/28
 description:
 	Check base version of full shell
 stdin:
@@ -14254,3 +14254,43 @@ expected-stdout:
 	2 eh .
 	3 eh .
 ---
+name: optional-printf-builtin
+description:
+	printf(1) minimal tests
+category: printf-builtin
+stdin:
+	t() {
+		o=$(\\builtin printf "$@"; echo .)
+		r=$?
+		print -r -- "$((++n)) ${o%.} : $r"
+	}
+	t '<%s>' a b\	c
+	t '%s - %c %s %d %i %o %u %x %X %b' 3
+	t '%% %c %s %d %i %o %u %x %X' 72 73 74 75 76 77 78 79
+	t '\\\a\b\f\n\r\t\v\01234'
+	let n+=2
+	t '%b foo %s bar' '\\\a\b\f\n\r\t\v\01234\cdefg'
+	let ++n
+	t '<%d>' 1 +2 -3 0x1a 0X1B 010 -011 +012 \'1 \"2
+	t '<%d|%d><%+d|%+d><% d|% d><%+ d|%+ d>' 1 -1 1 -1 1 -1 1 -1
+	t '<%o|%#o><%x|%#x><%X|%#X>' 8 9 10 11 12 13
+	#XXX replace with below
+	t '|%3d|%-3d|%+3d|%-+3d|%03d|%-03d|%3d|%.3d|notyet|' \
+	    1 2 3 4 5 6 7777 8
+	#t '|%3d|%-3d|%+3d|%-+3d|%03d|%-03d|%3d|%.3d|%5.3d|' \
+	#    1 2 3 4 5 6 7777 8 9
+	#12 |  1|2  | +3|+4 |005|6  |7777|008|  009| : 0
+expected-stdout:
+	1 <a><b	c> : 0
+	2 3 -   0 0 0 0 0 0  : 0
+	3 % 7 73 74 75 114 77 4e 4F : 0
+	4 \
+	
	
+	34 : 0
+	7 \
+	
	S4 : 0
+	9 <1><2><-3><26><27><8><-9><10><49><50> : 0
+	10 <1|-1><+1|-1>< 1|-1><+1|-1> : 0
+	11 <10|011><a|0xb><C|0XD> : 0
+	12 |  1|2  | +3|+4 |005|6  |7777|008|notyet| : 0
+---
diff -pruN mksh-59c-26/debian/changelog mksh-59c-28/debian/changelog
--- mksh-59c-26/debian/changelog	2023-02-15 16:04:53.000000000 +0100
+++ mksh-59c-28/debian/changelog	2023-04-28 23:34:20.000000000 +0200
@@ -1,3 +1,21 @@
+mksh (59c-28) unstable; urgency=medium
+
+  * Revert 59c-27 changes as mksh is, surprisingly, still a key
+    package for this release, shunit check-B-D
+  * Add cherry-picked individual fixes
+    - fix some formatting routine corner cases
+    - check the optional printf builtin (used for lksh) in the
+      testsuite, to avoid the issues reappearing
+  * Cherry-pick more individual fixes from upstream
+    - fix shift/rotate for nōn-power-of-two-sized bit quantities
+    - correct 59c regression in recursive parser for command
+      substitution and fix the other place it was not reentrant
+      as well (by moving function-static storage to stack/heap);
+      crash discovered by Riccardo Felici, USI Lugano
+  * Revert RCS ID changes, but assign individual version datestamp
+
+ -- Thorsten Glaser <tg@mirbsd.de>  Fri, 28 Apr 2023 23:34:20 +0200
+
 mksh (59c-26) unstable; urgency=medium
 
   * Fixup more update-shells breakage also found by piuparts
diff -pruN mksh-59c-26/mbsdint.h mksh-59c-28/mbsdint.h
--- mksh-59c-26/mbsdint.h	2023-05-16 19:13:21.000000000 +0200
+++ mksh-59c-28/mbsdint.h	2023-05-16 19:13:32.000000000 +0200
@@ -675,7 +675,7 @@ mbiCTAS(mbsdint_h) {
 #define mbiK_sr(ut,n,vl,vr,vz)	mbiK_SR(ut, mbiTYPE_UBITS(ut), n, vl, vr, vz)
 #define mbiMK_sr(ut,FM,n,l,r,z)	mbiMM(ut, (FM), mbiK_SR(ut, mbiMASK_BITS(FM), \
 				    n, mbiMM(ut, (FM), (l)), (r), (z)))
-#define mbiK_SR(ut,b,n,l,r,vz)	mbiK_RS(ut, b, n, l, mbiUI(r) & (b - 1U), (vz))
+#define mbiK_SR(ut,b,n,l,r,vz)	mbiK_RS(ut, b, n, l, mbiUI(r) % mbiUI(b), (vz))
 #define mbiK_RS(ut,b,n,v,cl,zx)	mbiOT(ut, cl, n(ut, v, cl, b - (cl), zx), v)
 #define mbiK_shl(ut,ax,cl,CL,z)	mbiOshl(ut, ax, cl)
 #define mbiK_shr(ut,ax,cl,CL,z)	mbiOshr(ut, ax, cl)
diff -pruN mksh-59c-26/sh.h mksh-59c-28/sh.h
--- mksh-59c-26/sh.h	2023-05-16 19:13:21.000000000 +0200
+++ mksh-59c-28/sh.h	2023-05-16 19:13:32.000000000 +0200
@@ -235,7 +235,7 @@
 #define __SCCSID(x)		__IDSTRING(sccsid,x)
 #endif
 
-#define MKSH_VERSION "R59 2023/01/31"
+#define MKSH_VERSION "R59 2023/04/28"
 
 /* shell types */
 typedef unsigned char kby;		/* byte */
@@ -2150,6 +2150,7 @@ struct ioword {
 #define IOBASH		BIT(10)	/* &> etc. */
 #define IOHERESTR	BIT(11)	/* <<< (here string) */
 #define IONDELIM	BIT(12)	/* null delimiter (<<) */
+#define IOSYNIONEXT	BIT(13)	/* already fully configured */
 
 /* execute/exchild flags */
 #define XEXEC	BIT(0)		/* execute without forking */
diff -pruN mksh-59c-26/shf.c mksh-59c-28/shf.c
--- mksh-59c-26/shf.c	2023-05-16 19:13:21.000000000 +0200
+++ mksh-59c-28/shf.c	2023-05-16 19:13:32.000000000 +0200
@@ -837,20 +837,15 @@ shf_smprintf(const char *fmt, ...)
 char *
 kslfmt(ksl number, kui flags, char *numbuf)
 {
+	/* easy for positive number */
+	if (number >= 0)
+		return (kulfmt((kul)number, flags, numbuf));
+	/* negative signed quantity */
 	if (!IS(flags, FM_TYPE, FL_SGN)) {
-		/* uh-oh, unsigned? what? be bitwise faithful here */
-		union {
-			/*XXX hopefully not UB… */
-			ksl s;
-			kul u;
-		} v;
-
-		v.s = number;
-		return (kulfmt(v.u, flags, numbuf));
+		/* uh-oh, output a signed quantity unsignedly */
+		return (kulfmt(KSL2UL(number), flags, numbuf));
 	}
-	if (number < 0)
-		flags |= FL_NEG;
-	return (kulfmt(KSL2NEGUL(number), flags, numbuf));
+	return (kulfmt(KSL2NEGUL(number), flags | FL_NEG, numbuf));
 }
 
 /* pre-initio() */
@@ -892,13 +887,13 @@ kulfmt(kul number, kui flags, char *numb
 			number /= 10UL;
 		} while (number);
 
-		if (!IS(flags, FM_TYPE, FL_DEC)) {
+		if (IS(flags, FM_TYPE, FL_SGN)) {
 			if (HAS(flags, FL_NEG))
 				*--cp = '-';
 			else if (HAS(flags, FL_PLUS))
 				*--cp = '+';
 			else if (HAS(flags, FL_BLANK))
-				*--cp = '-';
+				*--cp = ' ';
 		}
 		break;
 	}
diff -pruN mksh-59c-26/syn.c mksh-59c-28/syn.c
--- mksh-59c-26/syn.c	2023-05-16 19:13:21.000000000 +0200
+++ mksh-59c-28/syn.c	2023-05-16 19:13:32.000000000 +0200
@@ -191,19 +191,16 @@ static struct ioword *
 synio(int cf)
 {
 	struct ioword *iop;
-	static struct ioword *nextiop;
 	Wahr ishere;
 
-	if (nextiop != NULL) {
-		iop = nextiop;
-		nextiop = NULL;
-		return (iop);
-	}
-
 	if (tpeek(cf) != REDIR)
 		return (NULL);
 	ACCEPT;
 	iop = yylval.iop;
+	if (iop->ioflag & IOSYNIONEXT) {
+		iop->ioflag &= ~IOSYNIONEXT;
+		return (iop);
+	}
 	ishere = (iop->ioflag & IOTYPE) == IOHERE;
 	if (iop->ioflag & IOHERESTR) {
 		musthave(LWORD, 0);
@@ -228,17 +225,21 @@ synio(int cf)
 	if (iop->ioflag & IOBASH) {
 		char *cp;
 
-		nextiop = alloc(sizeof(*iop), ATEMP);
-		nextiop->ioname = cp = alloc(3, ATEMP);
+		iop->ioflag &= ~IOBASH;
+
+		cp = alloc(sizeof(struct ioword) + 3U, ATEMP);
+		yylval.iop = (void *)cp;
+		cp += sizeof(struct ioword);
+		yylval.iop->ioname = cp;
 		*cp++ = CHAR;
 		*cp++ = digits_lc[iop->unit];
 		*cp = EOS;
-
-		iop->ioflag &= ~IOBASH;
-		nextiop->unit = 2;
-		nextiop->ioflag = IODUP;
-		nextiop->delim = NULL;
-		nextiop->heredoc = NULL;
+		yylval.iop->delim = NULL;
+		yylval.iop->heredoc = NULL;
+		yylval.iop->ioflag = IODUP | IOSYNIONEXT;
+		yylval.iop->unit = 2;
+		REJECT;
+		symbol = REDIR;
 	}
 	return (iop);
 }
@@ -282,7 +283,7 @@ get_command(int cf, int sALIAS)
 	XPtrV args, vars;
 	struct nesting_state old_nesting;
 	Wahr check_decl_utility;
-	static struct ioword *iops[NUFILE + 1];
+	struct ioword *iops[NUFILE + 1];
 
 	XPinit(args, 16);
 	XPinit(vars, 16);

Reply to: