Package: release.debian.org
Severity: normal
User: release.debian....@packages.debian.org
Usertags: unblock
X-Debbugs-Cc: m...@packages.debian.org, t...@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 <t...@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 via email to