Buffer overflow with long fd numbers in redirects

[Mikael Magnusson <mikachu@gmail.com>]

Someone reported this on IRC the other day,
% >&333333333333333333333
zsh: number truncated after 20 digits: 333333333333333333333
*** buffer overflow detected ***: zsh terminated

At least one place where this is mishandled is in exec.c around line 3215,
        if (fil == -1) {
            char fdstr[4];

            closemnodes(mfds);
            fixfds(save);
            if (fn->fd2 != -2)
                sprintf(fdstr, "%d", fn->fd2);
            if (errno)
            zwarn("%s: %e", fn->fd2 == -2 ? "coprocess" : fdstr,
                  errno);
            execerr();
        }

Obviously anything over 999 will not fit in fdstr[]. I just checked
and it appears we do not use snprintf anywhere, is this for any
particular reason? The patch below just changes the array to [64], it
should be some time before any system uses a 256-bit type for fds. If
you guys have another preference for solving this, let me know. Note
however that just adding a check if (fn->fd2 != -2 && fn->fd2 < 1000
&& fn->fd2 > -100) is not sufficient since the zwarn attempts to use
fdstr for printing the error. (This is what I did first).

Output with the patch,
% >&333333333333333333333
zsh: number truncated after 20 digits: 333333333333333333333
zsh: 553997653: bad file descriptor

Arguably fdstr could be 21 because of that truncation but it would be
easy to miss if we lift that restriction at some point.

diff --git i/Src/exec.c w/Src/exec.c
index 499606f..906b6ca 100644
--- i/Src/exec.c
+++ w/Src/exec.c
@@ -3210,7 +3210,7 @@ execcmd()
                    fil = movefd(dup(fd));
                }
                if (fil == -1) {
-                   char fdstr[4];
+                   char fdstr[64];

                    closemnodes(mfds);
                    fixfds(save);


-- 
Mikael Magnusson