From 3d2b66cd3d2869fec16bce33361108103e20c10e Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Mon, 6 Jul 2020 01:35:36 -0700 Subject: [PATCH] Fix use of strdup on a NULL pointer 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 https://github.com/att/ast/issues/1028 src/cmd/ksh93/edit/history.c: - Make strdup duplicate 'notty' instead of NULL to prevent crashes. --- NEWS | 5 +++++ src/cmd/ksh93/edit/history.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index d55fd75fa36d..fe6b22f7e012 100644 --- a/NEWS +++ b/NEWS @@ -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 diff --git a/src/cmd/ksh93/edit/history.c b/src/cmd/ksh93/edit/history.c index d6737e209ca0..f40f27b4a4d7 100644 --- a/src/cmd/ksh93/edit/history.c +++ b/src/cmd/ksh93/edit/history.c @@ -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); } }