Skip to content

Commit

Permalink
Fix use of strdup on a NULL pointer
Browse files Browse the repository at this point in the history
The following set of commands can rarely cause a memory fault
when auditing is enabled, although most of the time it will
simply cause ksh to write '(null)' to the auditing file in place
of a tty name:

$ [ -e /etc/ksh_audit ] || echo "/tmp/ksh_auditfile;$(id -u)" | sudo tee /etc/ksh_audit;
$ v=$(ksh  2> /dev/null +o rc -ic $'getopts a:bc: opt --man\nprint $?')
$ cat /tmp/ksh_auditfile
1000;1593599493;(null); getopts a:bc: opt --man

This happens because strdup is used unconditionally on the pointer
returned by 'ttyname', which can be NULL if stderr is closed. The
ksh implementation of strdup then attempts to get the length of
NULL with strlen, which leads to undefined behavior.
See att#1028

src/cmd/ksh93/edit/history.c:
- Make strdup duplicate 'notty' instead of NULL to prevent
  crashes.
  • Loading branch information
JohnoKing committed Jul 6, 2020
1 parent 300cd19 commit 3d2b66c
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
5 changes: 5 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh

Any uppercase BUG_* names are modernish shell bug IDs.

2020-07-06:

- 'notty' is now written to the ksh auditing file instead of '(null)' if
the user's tty could not be determined.

2020-07-05:

- In UTF-8 locales, fix corruption of the shell's internal string quoting
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/edit/history.c
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ int sh_histinit(void *sh_context)
if(fd>=0)
{
fcntl(fd,F_SETFD,FD_CLOEXEC);
hp->tty = strdup(ttyname(2));
hp->tty = strdup(isatty(2)?ttyname(2):"notty");
hp->auditfp = sfnew((Sfio_t*)0,NULL,-1,fd,SF_WRITE);
}
}
Expand Down

0 comments on commit 3d2b66c

Please sign in to comment.