Skip to content

Commit

Permalink
{base,dir}name: add -z/--zero | arith: fix buffer overflow (#800)
Browse files Browse the repository at this point in the history
src/cmd/ksh93/sh/arith.c: sh_mathstdfun():
- Fixed small buffer overflow: The check for null termination needs
  to happen after ensuring the name matches because the strncmp
  will return false if the name is shorter than fsize.
- Remove a condition for breaking the loop that is never true;
  *tp->fname is a nonprintable byte that is never greater than c,
  which stores a printable character. See shtab_math in
  arch/*/src/cmd/ksh93/FEATURE/math.

src/lib/libcmd/basename.c:
- Added support for -z|--zero argument for compatiblity with GNU
  basename.
- Implement the behavior described in the usage string regarding
  PATH_LEADING_SLASHES (which it had not supported at all).

src/lib/libcmd/dirname.c:
- Fix handling of initial // where 'getconf PATH_LEADING_SLASHES'
  is 1 (i.e. Cygwin).
- Added support for -z|--zero argument for compatiblity with GNU
  basename.
- Handle multiple operands.

Co-authored-by: Martijn Dekker <martijn@inlv.org>
  • Loading branch information
dnewhall and McDutchie authored Dec 26, 2024
1 parent bb83575 commit c692ee1
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 26 deletions.
13 changes: 13 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@ This documents significant changes in the dev branch of ksh 93u+m.
For full details, see the git log at: https://github.com/ksh93/ksh
Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.

2024-12-25:

- The dirname path-bound built-in now accepts multiple operands.

- The basename and dirname path-bound built-ins now have a -z/--zero option
for compatibility with the GNU versions of these utilities. It causes the
line terminator to be a zero byte instead of a newline.

- Fixed bugs related to the handling of an initial double slash in the
basename and dirname path-bound built-ins when used on Windows (Cygwin).

- Fixed a buffer overflow problem in the handling of arithmetic functions.

2024-12-24:

- [v1.1] The fc/hist command has a new -E option. If this option is given,
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include <ast_release.h>
#include "git.h"

#define SH_RELEASE_DATE "2024-12-24" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_DATE "2024-12-25" /* must be in this format for $((.sh.version)) */
/*
* This comment keeps SH_RELEASE_DATE a few lines away from SH_RELEASE_SVER to avoid
* merge conflicts when cherry-picking dev branch commits onto a release branch.
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/ksh93/sh/arith.c
Original file line number Diff line number Diff line change
Expand Up @@ -198,15 +198,15 @@ static Namval_t *scope(Namval_t *np,struct lval *lvalue,int assign)
return np;
}

/* look up a function in the standard math function table */
static Math_f sh_mathstdfun(const char *fname, size_t fsize, short * nargs)
{
const struct mathtab *tp;
char c = fname[0];
/* first byte of tp->fname is num. args and return type, unless empty */
for(tp=shtab_math; *tp->fname; tp++)
{
if(*tp->fname > c)
break;
if(tp->fname[1]==c && tp->fname[fsize+1]==0 && strncmp(&tp->fname[1],fname,fsize)==0)
if(tp->fname[1]==c && strncmp(&tp->fname[1],fname,fsize)==0 && tp->fname[fsize+1]==0)
{
if(nargs)
*nargs = *tp->fname;
Expand Down
14 changes: 14 additions & 0 deletions src/cmd/ksh93/tests/libcmd.sh
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,14 @@ if builtin basename 2> /dev/null; then
got=$(basename "$tmp/.bar")
exp=".bar"
[[ $got == "$exp" ]] || err_exit "basename failed (expected $(printf %q "$exp"), got $(printf %q "$got"))"

# PATH_LEADING_SLASHES handling
case $(builtin getconf 2>/dev/null && getconf PATH_LEADING_SLASHES 2>/dev/null) in
1) exp=$'/\n//\n//\n//' ;;
*) exp=$'/\n/\n/\n/' ;;
esac
got=$(basename -a / // /// ////)
[[ $got == "$exp" ]] || err_exit "PATH_LEADING_SLASHES handling (expected $(printf %q "$exp"), got $(printf %q "$got"))"
fi

# ======
Expand Down Expand Up @@ -840,6 +848,12 @@ if builtin dirname 2> /dev/null; then
# Print the $PATH relative executable file path for string.
got=$(dirname -x cat)
[[ $got == "$exp" ]] || err_exit "dirname -x failed (expected $(printf %q "$exp"), got $(printf %q "$got"))"

# PATH_LEADING_SLASHES handling; multiple operands
exp=$'/\n//\n//\n//\n//\n//server\n//server'
got=$(export _AST_FEATURES='PATH_LEADING_SLASHES - 1'
"$SHELL" -c 'builtin dirname && dirname / // // //server ///server //server/name ///server//name' 2>&1)
[[ $got == "$exp" ]] || err_exit "PATH_LEADING_SLASHES handling (expected $(printf %q "$exp"), got $(printf %q "$got"))"
fi

# ======
Expand Down
28 changes: 19 additions & 9 deletions src/lib/libcmd/basename.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
*/

static const char usage[] =
"[-?\n@(#)$Id: basename (ksh 93u+m) 2022-08-30 $\n]"
"[-?\n@(#)$Id: basename (ksh 93u+m) 2024-12-05 $\n]"
"[--catalog?" ERROR_CATALOG "]"
"[+NAME?basename - strip directory and suffix from filenames]"
"[+DESCRIPTION?\bbasename\b removes all leading directory components "
Expand All @@ -48,6 +48,8 @@ static const char usage[] =
"[s:suffix?All operands are treated as \astring\a and each modified "
"pathname, with \asuffix\a removed if it exists, is printed on a "
"separate line on the standard output.]:[suffix]"
"[z:zero?Each line of output is terminated with a NUL character instead "
"of a newline.]"
"\n"
"\n string [suffix]\n"
"string ...\n"
Expand All @@ -63,23 +65,27 @@ static const char usage[] =

#include <cmd.h>

static void namebase(Sfio_t *outfile, char *pathname, char *suffix)
static void namebase(Sfio_t *outfile, char *pathname, char *suffix, char termch)
{
char *first, *last;
int n=0;
/* go to end of path */
for(first=last=pathname; *last; last++);
/* back over trailing '/' */
if(last>first)
while(*--last=='/' && last > first);
/* only slash(es)? */
if(last==first && *last=='/')
{
/* all '/' or "" */
if(*first=='/')
if(*++last=='/') /* keep leading // */
last++;
/* advance back over first '/' */
last++;
/* keep leading '//' if PATH_LEADING_SLASHES is set */
if(*last=='/' && *astconf("PATH_LEADING_SLASHES",NULL,NULL)=='1')
last++;
}
else
{
/* set to first / from end */
for(first=last++;first>pathname && *first!='/';first--);
if(*first=='/')
first++;
Expand All @@ -92,7 +98,7 @@ static void namebase(Sfio_t *outfile, char *pathname, char *suffix)
}
if(last>first)
sfwrite(outfile,first,last-first);
sfputc(outfile,'\n');
sfputc(outfile,termch);
}

int
Expand All @@ -101,6 +107,7 @@ b_basename(int argc, char** argv, Shbltin_t* context)
char* string;
char* suffix = 0;
int all = 0;
char termch = '\n';

cmdinit(argc, argv, context, ERROR_CATALOG, 0);
for (;;)
Expand All @@ -114,6 +121,9 @@ b_basename(int argc, char** argv, Shbltin_t* context)
all = 1;
suffix = opt_info.arg;
continue;
case 'z':
termch = '\0';
continue;
case ':':
error(2, "%s", opt_info.arg);
break;
Expand All @@ -131,9 +141,9 @@ b_basename(int argc, char** argv, Shbltin_t* context)
UNREACHABLE();
}
if (!all)
namebase(sfstdout, argv[0], argv[1]);
namebase(sfstdout, argv[0], argv[1], termch);
else
while (string = *argv++)
namebase(sfstdout, string, suffix);
namebase(sfstdout, string, suffix, termch);
return 0;
}
40 changes: 27 additions & 13 deletions src/lib/libcmd/dirname.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@
*/

static const char usage[] =
"[-?\n@(#)$Id: dirname (AT&T Research) 2009-01-31 $\n]"
"[-?\n@(#)$Id: dirname (ksh 93u+m) 2024-12-25 $\n]"
"[--catalog?" ERROR_CATALOG "]"
"[+NAME?dirname - return directory portion of file name]"
"[+DESCRIPTION?\bdirname\b treats \astring\a as a file name and returns "
"[+DESCRIPTION?\bdirname\b treats each \astring\a as a file name and outputs "
"the name of the directory containing the file name by deleting "
"the last component from \astring\a.]"
"[+?If \astring\a consists solely of \b/\b characters the output will "
Expand All @@ -48,8 +48,10 @@ static const char usage[] =
"[f:file?Print the \b$PATH\b relative regular file path for \astring\a.]"
"[r:relative?Print the \b$PATH\b relative readable file path for \astring\a.]"
"[x:executable?Print the \b$PATH\b relative executable file path for \astring\a.]"
"[z:zero?Each line of output is terminated with a NUL character instead "
"of a newline.]"
"\n"
"\nstring\n"
"\nstring ...\n"
"\n"
"[+EXIT STATUS?]{"
"[+0?Successful completion.]"
Expand All @@ -60,7 +62,7 @@ static const char usage[] =

#include <cmd.h>

static void l_dirname(Sfio_t *outfile, const char *pathname)
static void l_dirname(Sfio_t *outfile, const char *pathname, char termch)
{
const char *last;
/* go to end of path */
Expand All @@ -80,23 +82,29 @@ static void l_dirname(Sfio_t *outfile, const char *pathname)
/* back over trailing '/' */
for(;*last=='/' && last > pathname; last--);
}
/* preserve // */
/* preserve leading '//' */
if(last==pathname && pathname[0]=='/')
while(last[1]=='/')
last++;
if(last!=pathname && pathname[0]=='/' && pathname[1]=='/')
{
/* skip any '/' until last two */
while(pathname[2]=='/' && pathname<last)
pathname++;
/* skip first '/' if PATH_LEADING_SLASHES not set */
if(last!=pathname && pathname[0]=='/' && pathname[1]=='/' && *astconf("PATH_LEADING_SLASHES",NULL,NULL)!='1')
pathname++;
}
sfwrite(outfile,pathname,last+1-pathname);
sfputc(outfile,'\n');
sfputc(outfile,termch);
}

int
b_dirname(int argc, char** argv, Shbltin_t* context)
{
int mode = 0;
char buf[PATH_MAX];
char termch = '\n';

cmdinit(argc, argv, context, ERROR_CATALOG, 0);
for (;;)
Expand All @@ -113,6 +121,9 @@ b_dirname(int argc, char** argv, Shbltin_t* context)
case 'x':
mode |= PATH_EXECUTE;
continue;
case 'z':
termch = '\0';
continue;
case ':':
error(2, "%s", opt_info.arg);
break;
Expand All @@ -124,16 +135,19 @@ b_dirname(int argc, char** argv, Shbltin_t* context)
}
argv += opt_info.index;
argc -= opt_info.index;
if(error_info.errors || argc != 1)
if(error_info.errors || argc < 1)
{
error(ERROR_usage(2),"%s", optusage(NULL));
UNREACHABLE();
}
if(!mode)
l_dirname(sfstdout,argv[0]);
else if(pathpath(argv[0], "", mode, buf, sizeof(buf)))
sfputr(sfstdout, buf, '\n');
else
error(1|ERROR_WARNING, "%s: relative path not found", argv[0]);
for(; argv[0]; argv++)
{
if(!mode)
l_dirname(sfstdout,argv[0],termch);
else if(pathpath(argv[0], "", mode, buf, sizeof(buf)))
sfputr(sfstdout, buf, termch);
else
error(1|ERROR_WARNING, "%s: relative path not found", argv[0]);
}
return 0;
}

0 comments on commit c692ee1

Please sign in to comment.