Skip to content

Commit a133fb7

Browse files
JohnoKingMcDutchie
authored andcommitted
Fix arbitrary command execution/code injection bugs (#866)
* Security patch part 1 Never use $SHELL or sh.shpath when executing shebang-less scripts - sh_ntfork(): Removed a misguided optimization that causes ksh to run scripts without a shebang using the binary pointed to by either $SHELL or sh.shpath. This has a few problems: - The shell in sh.shpath or more disastrously in $SHELL has no guarantee of being identical to the currently running copy of ksh93 or even existing at all, so using either is not only bogus, but potentially dangerous. - The optimization has no ability to pass down the current POSIX mode status to the script. - It's only activated for virtual subshells, resulting in arbitrarily different behavior depending on whether or not we're in a virtual subshell. - It does some weird stuff with /dev/fd that seems superfluous, and also lacks any SHOPT_DEVFD if directive. (Additionally, if it did have one, that stat(2) would likely become mere dead code and a waste of context switches.) The optimization was probably intended to be used for running a shebang-less script via posix_spawn, which is ostensibly faster than fork. But this simply isn't possible without risking running the shebang-less script in a shell environment different from the current one. (If ksh were updated by the package manager while ksh is still running, this optimization would cause the script to run via the new version, rather than the currently running older version.) The optimization is unfixable by design, and as such must be removed to ensure correct behavior. * Security patch part 2 (re: bae02c3) rm setuid script code leading to arbitrary command execution Changes: - Delete code for setuid scripts on "Solaris 2.5+" because it allows for arbitrary command execution. One millisecond you think you're launching ksh, the next you're at the mercy of a hijacker. Example: SHELL=/bin/ppmflash /bin/ksh -l /dev/fd/0 < <(true) MANPATH: usage: MANPATH flashfactor [ppmfile] flashfactor: 0.0 = original picture, 1.0 = total whiteout The pathshell() code doesn't *seem* to be vulnerable to privilege escalation, but who knows (cf. CVE-2019-14868; this might need its own CVE 2025. Maybe pathshell() should be scrapped entirely???) - Add fickle but functional regression test (you may need to pass KSH=/bin/ksh or some such to get it to fail against vulnerable versions of ksh). The test uses a login shell via the -l option, but the bug *does not* need a login shell. See: #874 (comment) Modify the execveat reproducer to pass along environ (which could include a hijacked SHELL), and you're in for a BAD time. Maybe the deleted code (introduced sometime within the period of 1995 and 1999) was relevant to some Solaris-specific use case or something. Maybe the erasure even causes an incompatibility. But that code must go; it's far too dangerous to execv whatever the hell pathshell gives us during **init**. (Need I bring CVE-2019-14868 back to remembrance again? This bug has similarities to that one.) FWIW, all of the regression tests in the ksh and modernish suites pass with this patch applied. * Security patch part 3 Delete pathshell() and replace uses of it with safer equivalents This function is a dangerous attack vector that ought not remain in the code base. The value returned by astconf() is doubtless safer than what is returned by pathshell(). * Other changes The libast pathprog function and prog feature test are now unused, and are removed.
1 parent 4e6d8a9 commit a133fb7

File tree

19 files changed

+53
-374
lines changed

19 files changed

+53
-374
lines changed

NEWS

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

5+
2025-06-14:
6+
7+
- Fixed a bug occurring on systems with posix_spawn(3), spawnve(2), and
8+
spawn(2), which when exploited could cause ksh93 to execute scripts
9+
without a shebang with a shell other than itself, provided /bin/ksh
10+
was hijacked, or in the case of ksh being unable to obtain it's own
11+
executable path, whatever the SHELL variable was set to.
12+
13+
- Fixed a bug that could cause scripts without a shebang to run
14+
without POSIX mode enabled when executed in a virtual subshell.
15+
16+
- Fixed a bug that could allow an attacker to hijack ksh93 during
17+
shell initialization to execv a shell other than ksh itself, leading to
18+
the ksh process being surreptitiously replaced with bash, zsh, or
19+
really anything, provided the binary name ends with the letters
20+
'sh'.
21+
522
2025-05-31:
623

724
- Fixed a bug that caused the multiline option to break when PS*

src/cmd/ksh93/include/shell.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,6 @@ struct sh_scoped
220220
char **trapcom; /* EXIT and signals */
221221
char **otrapcom; /* save parent EXIT and signals for v=$(trap) */
222222
struct Ufunction *real_fun; /* current 'function name' function */
223-
int repl_index;
224-
char *repl_arg;
225223
};
226224

227225
struct limits
@@ -264,7 +262,6 @@ struct Shell_s
264262
Namval_t *bltin_nodes;
265263
Namval_t *bltin_cmds;
266264
History_t *hist_ptr;
267-
char *shpath;
268265
char *user;
269266
char **sigmsg;
270267
char **login_files;

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-06-09" /* must be in this format for $((.sh.version)) */
21+
#define SH_RELEASE_DATE "2025-06-14" /* 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 & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,34 +1261,6 @@ Shell_t *sh_init(int argc,char *argv[], Shinit_f userinit)
12611261
}
12621262
/* increase SHLVL */
12631263
sh.shlvl++;
1264-
#if SHOPT_SPAWN
1265-
{
1266-
/*
1267-
* try to find the pathname for this interpreter
1268-
* try using environment variable _ or argv[0]
1269-
*/
1270-
char *cp=nv_getval(L_ARGNOD);
1271-
char buff[PATH_MAX+1];
1272-
size_t n;
1273-
sh.shpath = 0;
1274-
if((n = pathprog(NULL, buff, sizeof(buff))) > 0 && n <= sizeof(buff))
1275-
sh.shpath = sh_strdup(buff);
1276-
else if((cp && (sh_type(cp)&SH_TYPE_SH)) || (argc>0 && strchr(cp= *argv,'/')))
1277-
{
1278-
if(*cp=='/')
1279-
sh.shpath = sh_strdup(cp);
1280-
else if(cp = nv_getval(PWDNOD))
1281-
{
1282-
int offset = stktell(sh.stk);
1283-
sfputr(sh.stk,cp,'/');
1284-
sfputr(sh.stk,argv[0],-1);
1285-
pathcanon(stkptr(sh.stk,offset),PATH_DOTDOT);
1286-
sh.shpath = sh_strdup(stkptr(sh.stk,offset));
1287-
stkseek(sh.stk,offset);
1288-
}
1289-
}
1290-
}
1291-
#endif
12921264
nv_putval(IFSNOD,(char*)e_sptbnl,NV_RDONLY);
12931265
astconfdisc(newconf);
12941266
#if SHOPT_TIMEOUT
@@ -1301,7 +1273,6 @@ Shell_t *sh_init(int argc,char *argv[], Shinit_f userinit)
13011273
#endif
13021274
if(argc>0)
13031275
{
1304-
int dolv_index;
13051276
/* check for restricted shell */
13061277
if(type&SH_TYPE_RESTRICTED)
13071278
sh_onoption(SH_RESTRICTED);
@@ -1313,10 +1284,7 @@ Shell_t *sh_init(int argc,char *argv[], Shinit_f userinit)
13131284
sh_done(0);
13141285
}
13151286
opt_info.disc = 0;
1316-
dolv_index = (argc - 1) - sh.st.dolc;
1317-
sh.st.dolv = argv + dolv_index;
1318-
sh.st.repl_index = dolv_index;
1319-
sh.st.repl_arg = argv[dolv_index];
1287+
sh.st.dolv = argv + (argc - 1) - sh.st.dolc;
13201288
sh.st.dolv[0] = argv[0];
13211289
if(sh.st.dolc < 1)
13221290
{

src/cmd/ksh93/sh/main.c

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -221,33 +221,12 @@ noreturn void sh_main(int ac, char *av[], Shinit_f userinit)
221221
/* open stream should have been passed into shell */
222222
if(strmatch(name,e_devfdNN))
223223
{
224-
#if !_WINIX
225-
char *cp;
226-
int type;
227-
#endif
228224
fdin = (int)strtol(name+8, NULL, 10);
229225
if(fstat(fdin,&statb)<0)
230226
{
231227
errormsg(SH_DICT,ERROR_system(1),e_open,name);
232228
UNREACHABLE();
233229
}
234-
#if !_WINIX
235-
/*
236-
* try to undo effect of Solaris 2.5+
237-
* change for argv for setuid scripts
238-
*/
239-
if(sh.st.repl_index > 0)
240-
av[sh.st.repl_index] = sh.st.repl_arg;
241-
if(((type = sh_type(cp = av[0])) & SH_TYPE_SH) && (name = nv_getval(L_ARGNOD)) && (!((type = sh_type(cp = name)) & SH_TYPE_SH)))
242-
{
243-
av[0] = (type & SH_TYPE_LOGIN) ? cp : path_basename(cp);
244-
/* exec to change $0 for ps */
245-
execv(pathshell(),av);
246-
/* exec fails */
247-
sh.st.dolv[0] = av[0];
248-
fixargs(sh.st.dolv,1);
249-
}
250-
#endif
251230
name = av[0];
252231
sh_offoption(SH_VERBOSE);
253232
sh_offoption(SH_XTRACE);

src/cmd/ksh93/sh/path.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,8 +1221,6 @@ pid_t path_spawn(const char *opath,char **argv, char **envp, Pathcomp_t *libpath
12211221
*/
12221222
if(spawn)
12231223
{
1224-
if(sh.subshell)
1225-
return -1;
12261224
do
12271225
{
12281226
if((pid=fork())>0)

src/cmd/ksh93/sh/xec.c

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3306,26 +3306,6 @@ static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int topfd)
33063306
job_fork(-1);
33073307
jobfork = 1;
33083308
spawnpid = path_spawn(path,argv,arge,pp,(grp<<1)|1);
3309-
if(spawnpid < 0 && errno==ENOEXEC)
3310-
{
3311-
char *devfd;
3312-
int fd = open(path,O_RDONLY);
3313-
argv[-1] = argv[0];
3314-
argv[0] = path;
3315-
if(fd>=0)
3316-
{
3317-
struct stat statb;
3318-
sfprintf(sh.strbuf,"/dev/fd/%d",fd);
3319-
if(stat(devfd=sfstruse(sh.strbuf),&statb)>=0)
3320-
argv[0] = devfd;
3321-
}
3322-
if(!sh.shpath)
3323-
sh.shpath = pathshell();
3324-
spawnpid = path_spawn(sh.shpath,&argv[-1],arge,pp,(grp<<1)|1);
3325-
if(fd>=0)
3326-
close(fd);
3327-
argv[0] = argv[-1];
3328-
}
33293309
fail:
33303310
if(jobfork && spawnpid<0)
33313311
job_fork(-2);

src/cmd/ksh93/tests/basic.sh

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,5 +1074,33 @@ wait "$parallel_5" || err_exit 'command substitution causes pipefail option to h
10741074
wait "$parallel_6" || err_exit '"command | while read...done" finishing too fast'
10751075
wait "$parallel_7" || err_exit 'early termination not causing broken pipe'
10761076
1077+
# ======
1078+
# Hijacking ksh93 via $SHELL for arbitrary command execution during initialization.
1079+
# https://github.com/ksh93/ksh/issues/874
1080+
bindir=$tmp/dir.$RANDOM/bin
1081+
mkdir -p "$bindir"
1082+
echo $'#!/bin/sh\necho "CODE INJECTION"' > "$bindir"/hijack_sh
1083+
chmod +x "$bindir"/hijack_sh
1084+
got=$(set +x; SHELL="$bindir"/hijack_sh "$SHELL" -l <(echo) 2>&1)
1085+
[[ $got =~ "CODE INJECTION" ]] && err_exit 'ksh93 is vulnerable to being hijacked during init via $SHELL' \
1086+
"(got $(printf %q "$got"))"
1087+
1088+
# Hijacking ksh93 shebang-less scripts for arbitrary command execution.
1089+
# https://github.com/ksh93/ksh/pull/866
1090+
export bindir
1091+
print 'echo GOOD' > "$bindir/dummy.sh"
1092+
chmod +x "$bindir/dummy.sh"
1093+
cp "$SHELL" "$bindir/hijack_sh"
1094+
exp=$'GOOD\nGOOD'
1095+
got=$("$bindir/hijack_sh" -c $'print $\'\#!/bin/sh\necho HIJACKED\' > "$bindir/hijack_shell"
1096+
chmod +x "$bindir/hijack_shell"
1097+
rm "$bindir/hijack_sh"
1098+
cp "$bindir/hijack_shell" "$bindir/hijack_sh"
1099+
("$bindir/dummy.sh"); "$bindir/dummy.sh"; :')
1100+
rm -r "$bindir"
1101+
unset bindir
1102+
[[ $exp == $got ]] || err_exit 'ksh93 shebang-less scripts are vulnerable to being hijacked for arbitrary code execution' \
1103+
"(exp $(printf %q "$exp"), got $(printf %q "$got"))"
1104+
10771105
# ======
10781106
exit $((Errors<125?Errors:125))

src/cmd/ksh93/tests/posix.sh

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,13 @@ got=$(<out)
8383
[[ $got == "$exp" ]] || err_exit "incorrect --posix settings on invoking hashbangless script from posix shell (direct)" \
8484
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
8585

86-
: <<\DISABLED # TODO: these do not pass yet, unless SHOPT_SPAWN is disabled.
8786
(./script) >|out
8887
got=$(<out)
8988
[[ $got == "$exp" ]] || err_exit "incorrect --posix settings on invoking hashbangless script from posix shell (subshell)" \
9089
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
9190
got=$(./script)
9291
[[ $got == "$exp" ]] || err_exit "incorrect --posix settings on invoking hashbangless script from posix shell (comsub)" \
9392
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
94-
DISABLED
9593

9694
got=$("$SHELL" --posix -c "$(<script)")
9795
[[ $got == "$exp" ]] || err_exit "incorrect --posix settings on invoking -c script from posix shell" \

src/lib/libast/Mamfile

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -322,11 +322,6 @@ make install virtual
322322
exec - invoke_iffe %{<}
323323
done
324324

325-
make FEATURE/prog
326-
makp features/prog
327-
exec - invoke_iffe %{<}
328-
done
329-
330325
make FEATURE/locale
331326
makp features/locale
332327
exec - invoke_iffe %{<}
@@ -1216,13 +1211,6 @@ make install virtual
12161211
exec - compile %{<}
12171212
done
12181213

1219-
make pathshell.o
1220-
make path/pathshell.c
1221-
prev include/ast.h
1222-
done
1223-
exec - compile %{<}
1224-
done
1225-
12261214
make pathcd.o
12271215
make path/pathcd.c
12281216
prev include/stk.h
@@ -1232,15 +1220,6 @@ make install virtual
12321220
exec - compile %{<}
12331221
done
12341222

1235-
make pathprog.o
1236-
make path/pathprog.c
1237-
prev FEATURE/prog
1238-
prev include/ast_windows.h
1239-
prev include/ast.h
1240-
done
1241-
exec - compile %{<}
1242-
done
1243-
12441223
make ftwalk.o
12451224
make misc/ftwalk.c
12461225
make include/ftwalk.h

0 commit comments

Comments
 (0)