Skip to content

Commit b3ce22b

Browse files
committed
Fix /dev/fd/1 memory fault and $0 for /dev/fd scripts
Changes: - sh_init(): Don't set $0 to the path to ksh. The correct behavior is to always use the script name, even when it's a /dev/fd file. The behavior of ksh93 should match that of bash and other shells. Please read: ksh93#866 (comment) ksh93#874 (comment) - sh_main(): Do not attempt to optimize away sh_open() for the stdio fds (0, 1, 2) because that can cause buggy behavior. Ksh's handling of /dev/fd/1 and /dev/stdin should not differ. See: ksh93#879 (comment) - exfile(): Don't dup or close fds for /dev/fd scripts with a file descriptor > 2. Those should be left open pursuant to the $0 name of the script. (This cannot be done safely for fds 1 and 2, so the stdio fds are all dup'ed, but not closed.) - sh_main(): If the fd obtained for the script by sh_open() is <= 2, that means the shell was opened with one of the stdio fds closed, so for correct behavior use sh_iomovefd() to keep the stdio fd closed. - sh_iomovefd(): As an optimization, move the fd to a number that's either >= 3 or >= 10, depending on what is most appropriate. This avoids a possible inefficient scenario of dup'ing an fd from 0 -> 3 -> 10 (less syscalls is better for performance). - sh_main(): Use O_CLOEXEC in advance for the fd (which is guaranteed to get close-on-exec via a later fcntl call; if we don't end up calling fcntl for whatever reason O_CLOEXEC will help avoid an extra syscall). - basic.sh: Add regression tests for $0 and its associated fd. For the time being I've omitted a /dev/fd/1 regression test because I can't get one to work correctly with the pty tool. Patch originally posted here: ksh93#879 (comment) Fixes ksh93#874 Fixes ksh93#877
1 parent d2a0f1d commit b3ce22b

File tree

8 files changed

+94
-38
lines changed

8 files changed

+94
-38
lines changed

NEWS

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,20 @@ This documents significant changes in the dev branch of ksh 93u+m.
22
For full details, see the git log at: https://github.com/ksh93/ksh
33
Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
44

5+
2025-07-11:
6+
7+
- Fixed a bug that caused /dev/fd scripts to use the ksh binary for $0
8+
rather than the actual /dev/fd file path to the script.
9+
10+
- Fixed a bug that could cause the file descriptor for /dev/fd scripts
11+
to be closed during init.
12+
13+
- Fixed a bug that caused the inherited file descriptor for /dev/fd
14+
scripts to wrongly get marked as close-on-exec.
15+
16+
- Fixed a bug that could cause ksh to reopen stdout or stderr during
17+
init in spite of a 1>&- or 2>&- redirect.
18+
519
2025-07-10:
620

721
- Fixed a file descriptor leak introduced in 93u+ 2012-07-27 that occurred

src/cmd/ksh93/include/io.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@
7676

7777
extern int sh_iocheckfd(int);
7878
extern void sh_ioinit(void);
79-
extern int sh_iomovefd(int);
79+
extern int sh_iomovefd(int,int);
8080
extern int sh_iorenumber(int,int);
8181
extern void sh_pclose(int[]);
8282
extern int sh_rpipe(int[],int);

src/cmd/ksh93/include/version.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
#include <ast_release.h>
1919
#include "git.h"
2020

21-
#define SH_RELEASE_DATE "2025-07-10" /* must be in this format for $((.sh.version)) */
21+
#define SH_RELEASE_DATE "2025-07-11" /* must be in this format for $((.sh.version)) */
2222
/*
2323
* This comment keeps SH_RELEASE_DATE a few lines away from SH_RELEASE_SVER to avoid
2424
* merge conflicts when cherry-picking dev branch commits onto a release branch.

src/cmd/ksh93/sh/init.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,10 +1347,7 @@ Shell_t *sh_init(int argc,char *argv[], Shinit_f userinit)
13471347
else
13481348
sh_offoption(SH_PRIVILEGED);
13491349
/* shname for $0 in profiles and . scripts */
1350-
if(sh_isdevfd(argv[1]))
1351-
sh.shname = sh_strdup(argv[0]);
1352-
else
1353-
sh.shname = sh_strdup(sh.st.dolv[0]);
1350+
sh.shname = sh_strdup(sh.st.dolv[0]);
13541351
/*
13551352
* return here for shell script execution
13561353
* but not for parenthesis subshells

src/cmd/ksh93/sh/io.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -897,20 +897,20 @@ int sh_chkopen(const char *name)
897897
}
898898

899899
/*
900-
* move open file descriptor to a number > 2
900+
* move open file descriptor to a number >= minfd
901901
*/
902-
int sh_iomovefd(int fdold)
902+
int sh_iomovefd(int fdold, int minfd)
903903
{
904904
int fdnew, dupflags;
905905
if(fdold >= sh.lim.open_max)
906906
sh_iovalidfd(fdold);
907-
if(fdold<0 || fdold>2)
907+
if(fdold<0 || fdold>=minfd)
908908
return fdold;
909909
if(sh.fdstatus[fdold]&IOCLEX)
910910
dupflags = F_dupfd_cloexec;
911911
else
912912
dupflags = F_DUPFD;
913-
fdnew = sh_iomovefd(fcntl(fdold,dupflags,3));
913+
fdnew = sh_iomovefd(fcntl(fdold,dupflags,minfd),minfd);
914914
if((sh.fdstatus[fdold]&IOCLEX) && F_dupfd_cloexec == F_DUPFD)
915915
fcntl(fdnew,F_SETFD,FD_CLOEXEC);
916916
sh.fdstatus[fdnew] = sh.fdstatus[fdold];
@@ -946,9 +946,9 @@ int sh_pipe(int pv[], int cloexec)
946946
sh.fdstatus[pv[0]] = IONOSEEK|IOREAD|cloexec;
947947
sh.fdstatus[pv[1]] = IONOSEEK|IOWRITE|cloexec;
948948
if(pv[0]<=2)
949-
pv[0] = sh_iomovefd(pv[0]);
949+
pv[0] = sh_iomovefd(pv[0],3);
950950
if(pv[1]<=2)
951-
pv[1] = sh_iomovefd(pv[1]);
951+
pv[1] = sh_iomovefd(pv[1],3);
952952
sh_subsavefd(pv[0]);
953953
sh_subsavefd(pv[1]);
954954
return 0;
@@ -978,9 +978,9 @@ int sh_rpipe(int pv[], int cloexec)
978978
sh.fdstatus[pv[0]] = IONOSEEK|IOREAD|cloexec;
979979
sh.fdstatus[pv[1]] = IONOSEEK|IOWRITE|cloexec;
980980
if(pv[0]<=2)
981-
pv[0] = sh_iomovefd(pv[0]);
981+
pv[0] = sh_iomovefd(pv[0],3);
982982
if(pv[1]<=2)
983-
pv[1] = sh_iomovefd(pv[1]);
983+
pv[1] = sh_iomovefd(pv[1],3);
984984
sh_subsavefd(pv[0]);
985985
sh_subsavefd(pv[1]);
986986
return 0;
@@ -1537,7 +1537,7 @@ int sh_redirect(struct ionod *iop, int flag)
15371537
sh_close(fn);
15381538
}
15391539
if(flag==3)
1540-
return sh_iomovefd(fd); /* ensure FD > 2 to make $(<file) work with std{in,out,err} closed */
1540+
return sh_iomovefd(fd,3); /* ensure FD > 2 to make $(<file) work with std{in,out,err} closed */
15411541
if(fd>=0)
15421542
{
15431543
if(np)
@@ -1564,7 +1564,7 @@ int sh_redirect(struct ionod *iop, int flag)
15641564
}
15651565
else
15661566
{
1567-
fd = sh_iorenumber(sh_iomovefd(fd),fn);
1567+
fd = sh_iorenumber(sh_iomovefd(fd,3),fn);
15681568
if(fn>2 && fn<10)
15691569
sh.inuse_bits |= (1<<fn);
15701570
}

src/cmd/ksh93/sh/main.c

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ static struct stat lastmail;
6464
static time_t mailtime;
6565
static char beenhere = 0;
6666

67+
#if SHOPT_DEVFD
68+
#define GOT_DEVFD 2
69+
#endif
70+
6771
/*
6872
* search for file and exfile() it if it exists
6973
* 1 returned if file found, 0 otherwise
@@ -125,7 +129,7 @@ noreturn void sh_main(int ac, char *av[], Shinit_f userinit)
125129
#endif
126130
if(!beenhere)
127131
{
128-
beenhere++;
132+
beenhere = 1;
129133
sh_onstate(SH_PROFILE);
130134
sh.sigflag[SIGTSTP] |= SH_SIGIGNORE;
131135
/* decide whether shell is interactive */
@@ -218,11 +222,10 @@ noreturn void sh_main(int ac, char *av[], Shinit_f userinit)
218222
fdin = 0;
219223
else
220224
{
221-
char *sp;
225+
#if SHOPT_DEVFD
222226
/* open stream should have been passed into shell */
223-
if(strmatch(name,e_devfdNN))
227+
if(strmatch(name,e_devfdNN) && (fdin=(int)strtol(name+8, NULL, 10)) > 2)
224228
{
225-
fdin = (int)strtol(name+8, NULL, 10);
226229
if(fstat(fdin,&statb)<0)
227230
{
228231
errormsg(SH_DICT,ERROR_system(1),e_open,name);
@@ -231,11 +234,14 @@ noreturn void sh_main(int ac, char *av[], Shinit_f userinit)
231234
name = av[0];
232235
sh_offoption(SH_VERBOSE);
233236
sh_offoption(SH_XTRACE);
237+
beenhere = GOT_DEVFD;
234238
}
235239
else
240+
#endif /* SHOPT_DEVFD */
236241
{
242+
char *sp;
237243
int isdir = 0;
238-
if((fdin=sh_open(name,O_RDONLY,0))>=0 &&(fstat(fdin,&statb)<0 || S_ISDIR(statb.st_mode)))
244+
if((fdin=sh_open(name,O_RDONLY|O_cloexec,0))>=0 &&(fstat(fdin,&statb)<0 || S_ISDIR(statb.st_mode)))
239245
{
240246
sh_close(fdin);
241247
isdir = 1;
@@ -250,7 +256,7 @@ noreturn void sh_main(int ac, char *av[], Shinit_f userinit)
250256
sp = stkptr(sh.stk,PATH_OFFSET);
251257
if(sp)
252258
{
253-
if((fdin=sh_open(sp,O_RDONLY,0))>=0)
259+
if((fdin=sh_open(sp,O_RDONLY|O_cloexec,0))>=0)
254260
sh.st.filename = path_fullname(sp);
255261
}
256262
}
@@ -272,8 +278,14 @@ noreturn void sh_main(int ac, char *av[], Shinit_f userinit)
272278
strcopy(name," \"$@\"");
273279
goto shell_c;
274280
}
275-
if(fdin==0)
276-
fdin = sh_iomovefd(fdin);
281+
/*
282+
* Note: fdin could wind up being zero or some such if the
283+
* shell was opened with stdin/stdout/stderr closed
284+
* (i.e. 0>&-), so we move the fd in such a case to
285+
* keep that file descriptor closed.
286+
*/
287+
if(fdin<=2)
288+
fdin = sh_iomovefd(fdin,10);
277289
}
278290
sh.readscript = sh.shname;
279291
}
@@ -347,28 +359,38 @@ static void exfile(Sfio_t *iop,int fno)
347359
{
348360
if(fno > 0)
349361
{
350-
if(fno < 10)
362+
#if SHOPT_DEVFD
363+
if(beenhere == GOT_DEVFD)
364+
; /* leave the inherited /dev/fd as is */
365+
else
366+
#endif
351367
{
352-
int r = sh_fcntl(fno,F_dupfd_cloexec,10);
353-
if(r >= 10)
368+
if(fno < 10)
354369
{
355-
if(F_dupfd_cloexec != F_DUPFD)
356-
sh.fdstatus[r] = sh.fdstatus[fno]|IOCLEX;
357-
else
358-
sh.fdstatus[r] = sh.fdstatus[fno];
359-
sh_close(fno);
360-
fno = r;
370+
int r = sh_fcntl(fno,F_dupfd_cloexec,10);
371+
if(r >= 10)
372+
{
373+
if(F_dupfd_cloexec != F_DUPFD)
374+
sh.fdstatus[r] = sh.fdstatus[fno]|IOCLEX;
375+
else
376+
sh.fdstatus[r] = sh.fdstatus[fno];
377+
/* leave std* fds open when they're not explicitly closed */
378+
if(fno > 2)
379+
sh_close(fno);
380+
fno = r;
381+
}
361382
}
383+
if(!(sh.fdstatus[fno]&IOCLEX))
384+
sh_fcntl(fno,F_SETFD,FD_CLOEXEC);
362385
}
363-
if(!(sh.fdstatus[fno]&IOCLEX))
364-
sh_fcntl(fno,F_SETFD,FD_CLOEXEC);
365386
iop = sh_iostream(fno);
366387
}
367388
else
368389
iop = sfstdin;
369390
}
370391
else
371392
fno = -1;
393+
beenhere = 1;
372394
sh.infd = fno;
373395
if(sh_isstate(SH_INTERACTIVE))
374396
{

src/cmd/ksh93/sh/path.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ static int opentype(const char *name, Pathcomp_t *pp, int fun)
489489
}
490490
}
491491
while(fd<0 && nextpp);
492-
if(fd>=0 && (fd = sh_iomovefd(fd)) > 0 && !(sh.fdstatus[fd]&IOCLEX))
492+
if(fd>=0 && (fd = sh_iomovefd(fd,10)) > 0 && !(sh.fdstatus[fd]&IOCLEX))
493493
sh_fcntl(fd,F_SETFD,FD_CLOEXEC);
494494
return fd;
495495
}
@@ -1325,7 +1325,7 @@ static noreturn void exscript(char *path,char *argv[])
13251325
errormsg(SH_DICT,ERROR_system(ERROR_NOEXEC),e_exec,path);
13261326
UNREACHABLE();
13271327
}
1328-
sh.infd = sh_iomovefd(sh.infd);
1328+
sh.infd = sh_iomovefd(sh.infd,10);
13291329
#if SHOPT_ACCT
13301330
sh_accbegin(path) ; /* reset accounting */
13311331
#endif /* SHOPT_ACCT */

src/cmd/ksh93/tests/basic.sh

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1134,7 +1134,30 @@ cp "$bindir/hijack_shell" "$bindir/hijack_sh"
11341134
rm -r "$bindir"
11351135
unset bindir
11361136
[[ $exp == $got ]] || err_exit 'ksh93 shebang-less scripts are vulnerable to being hijacked for arbitrary code execution' \
1137-
"(exp $(printf %q "$exp"), got $(printf %q "$got"))"
1137+
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
1138+
1139+
# ======
1140+
# $0 should be the /dev/fd path for scripts executed from a /dev/fd file
1141+
# https://github.com/ksh93/ksh/issues/874
1142+
# https://github.com/ksh93/ksh/pull/879
1143+
if ((SHOPT_DEVFD))
1144+
then # $0 should be the /dev/fd script name when the script is a process substitution
1145+
got=$("$SHELL" <(echo 'echo $0'))
1146+
if [[ ${got:0:7} != '/dev/fd' ]]
1147+
then err_exit '$0 is wrong for process substitution scripts' \
1148+
"(expected a /dev/fd file name, got $(printf %q "$got"))"
1149+
else # The file descriptor for the /dev/fd script must remain open
1150+
if ! "$SHELL" <(echo '[[ -e $0 ]]')
1151+
then err_exit 'the file descriptor corresponding to $0 is not open in /dev/fd scripts'
1152+
fi
1153+
# The file descriptor for the /dev/fd script should not be close-on-exec
1154+
typeset -x verify_fd_script=$tmp/verify-procsub-$RANDOM.ksh
1155+
echo '[[ -e $fd ]]' > "$verify_fd_script"
1156+
if ! "$SHELL" <(echo 'typeset -x fd=$0; "$SHELL" "$verify_fd_script"')
1157+
then err_exit 'file descriptor for /dev/fd script is not passed on to child processes'
1158+
fi
1159+
fi
1160+
fi
11381161
11391162
# ======
11401163
exit $((Errors<125?Errors:125))

0 commit comments

Comments
 (0)