summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMikael Magnusson <mikachu@gmail.com>2026-05-08 15:12:57 +0200
committerMikael Magnusson <mikachu@gmail.com>2026-05-25 07:05:09 +0200
commit734a26d6ae88a436d477bf1855c90fcb4b581320 (patch)
tree3021ffafe8173fd3e19258b0354fbdfc34dd65d3
parent54593: fix two edge cases in join_strs (diff)
downloadzsh-734a26d6ae88a436d477bf1855c90fcb4b581320.tar
zsh-734a26d6ae88a436d477bf1855c90fcb4b581320.tar.gz
zsh-734a26d6ae88a436d477bf1855c90fcb4b581320.tar.bz2
zsh-734a26d6ae88a436d477bf1855c90fcb4b581320.tar.lz
zsh-734a26d6ae88a436d477bf1855c90fcb4b581320.tar.xz
zsh-734a26d6ae88a436d477bf1855c90fcb4b581320.tar.zst
zsh-734a26d6ae88a436d477bf1855c90fcb4b581320.zip
54594: fix two very similar coverity issues
Coverity CID 1255838 stack buffer overflow in execute() Coverity CID 1255846 stack buffer overflow in findcmd() This can be triggered by inserting long entries manually, then using =foo expansion. Unfortunately, it is also possible to overflow buf by importing a PATH entry that's over 4xPATH_MAX characters long. There was also some related confusion about how long a path can be and when/how to check the length of metafied strings, so fix all that up too. Use MAXCMDLEN (which is PATH_MAX*4) for metafied strings, though this is a little overkill, metafication can only double the length. We do not at least need the +1 at the end. PATH_MAX includes the terminating nul byte, so we make sure to not be off-by-one on our ztrlen checks too. The hunks in execute() are quite specific about the ztrlen length so it can continue checking or return the correct error, while findcmd() just relies on RET_IF_COM to work correctly instead. Somewhat unrelated change that snuck in, return 127 when the command is too long like the other exec failures, because I had to add a new return anyway.
-rw-r--r--ChangeLog2
-rw-r--r--Src/exec.c82
2 files changed, 58 insertions, 26 deletions
diff --git a/ChangeLog b/ChangeLog
index 857da0495..fc5c72299 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,7 @@
2026-05-25 Mikael Magnusson <mikachu@gmail.com>
+ * 54594: Src/exec.c: fix two very similar coverity issues
+
* 54593: Src/Zle/compmatch.c: fix two edge cases in join_strs
* 54592: Src/Zle/computil.c, Src/module.c: fix two dead
diff --git a/Src/exec.c b/Src/exec.c
index 2c730b910..cff688a2a 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -724,7 +724,7 @@ static void
execute(LinkList args, int flags, int defpath)
{
Cmdnam cn;
- char buf[MAXCMDLEN+1], buf2[MAXCMDLEN+1];
+ char buf[MAXCMDLEN], buf2[MAXCMDLEN];
char *s, *z, *arg0;
char **argv, **pp, **newenvp = NULL;
int eno = 0, ee;
@@ -785,9 +785,9 @@ execute(LinkList args, int flags, int defpath)
}
#endif
child_unblock();
- if ((int) strlen(arg0) >= PATH_MAX) {
+ if ((int) ztrlen(arg0) >= PATH_MAX) {
zerr("command too long: %s", arg0);
- _exit(1);
+ _exit(127);
}
for (s = arg0; *s; s++)
if (*s == '/') {
@@ -803,10 +803,10 @@ execute(LinkList args, int flags, int defpath)
/* for command -p, search the default path */
if (defpath) {
- char pbuf[PATH_MAX+1];
+ char pbuf[MAXCMDLEN];
char *dptr;
- if (!search_defpath(arg0, pbuf, PATH_MAX)) {
+ if (!search_defpath(arg0, pbuf, MAXCMDLEN)) {
if (commandnotfound(arg0, args) == 0)
_realexit();
zerr("command not found: %s", arg0);
@@ -823,28 +823,43 @@ execute(LinkList args, int flags, int defpath)
} else {
if ((cn = (Cmdnam) cmdnamtab->getnode(cmdnamtab, arg0))) {
- char nn[PATH_MAX+1], *dptr;
+ char nn[MAXCMDLEN], *dptr;
- if (cn->node.flags & HASHED)
- strcpy(nn, cn->u.cmd);
- else {
- for (pp = path; pp < cn->u.name; pp++)
+ if (cn->node.flags & HASHED) {
+ if (snprintf(nn, sizeof(nn), "%s", cn->u.cmd) >= (int)sizeof(nn) ||
+ ztrlen(nn) >= PATH_MAX)
+ {
+ zerr("command too long: %s", arg0);
+ _exit(127);
+ }
+ } else {
+ for (pp = path; pp < cn->u.name; pp++) {
if (!**pp || (**pp == '.' && (*pp)[1] == '\0')) {
ee = zexecve(arg0, argv, newenvp);
if (isgooderr(ee, *pp))
eno = ee;
} else if (**pp != '/') {
z = buf;
- strucpy(&z, *pp);
+ struncpy(&z, *pp, sizeof(buf) - 1);
*z++ = '/';
+ if ((z - buf) + strlen(arg0) >= sizeof(buf))
+ continue;
strcpy(z, arg0);
+ if (ztrlen(buf) >= PATH_MAX)
+ continue;
ee = zexecve(buf, argv, newenvp);
if (isgooderr(ee, *pp))
eno = ee;
}
- strcpy(nn, cn->u.name ? *(cn->u.name) : "");
- strcat(nn, "/");
- strcat(nn, cn->node.nam);
+ }
+ if (snprintf(nn, sizeof(nn), "%s/%s",
+ cn->u.name ? *(cn->u.name) : "",
+ cn->node.nam) >= (int)sizeof(nn) ||
+ ztrlen(nn) >= PATH_MAX)
+ {
+ eno = ENAMETOOLONG;
+ goto execute_skip_exec;
+ }
}
ee = zexecve(nn, argv, newenvp);
@@ -853,6 +868,7 @@ execute(LinkList args, int flags, int defpath)
if (isgooderr(ee, *nn ? nn : "/"))
eno = ee;
}
+execute_skip_exec:
for (pp = path; *pp; pp++)
if (!(*pp)[0] || ((*pp)[0] == '.' && !(*pp)[1])) {
ee = zexecve(arg0, argv, newenvp);
@@ -860,9 +876,13 @@ execute(LinkList args, int flags, int defpath)
eno = ee;
} else {
z = buf;
- strucpy(&z, *pp);
+ struncpy(&z, *pp, sizeof(buf) - 1);
*z++ = '/';
+ if ((z - buf) + strlen(arg0) >= sizeof(buf))
+ continue;
strcpy(z, arg0);
+ if (ztrlen(buf) >= PATH_MAX)
+ continue;
ee = zexecve(buf, argv, newenvp);
if (isgooderr(ee, *pp))
eno = ee;
@@ -910,7 +930,7 @@ findcmd(char *arg0, int docopy, int default_path)
cn = (Cmdnam) cmdnamtab->getnode(cmdnamtab, arg0);
if (!cn && isset(HASHCMDS) && !isrelative(arg0))
cn = hashcmd(arg0, path);
- if ((int) strlen(arg0) > PATH_MAX)
+ if ((int) ztrlen(arg0) >= PATH_MAX)
return NULL;
if ((s = strchr(arg0, '/'))) {
RET_IF_COM(arg0);
@@ -920,32 +940,42 @@ findcmd(char *arg0, int docopy, int default_path)
}
}
if (cn) {
- char nn[PATH_MAX+1];
+ char nn[MAXCMDLEN];
- if (cn->node.flags & HASHED)
- strcpy(nn, cn->u.cmd);
- else {
- for (pp = path; pp < cn->u.name; pp++)
+ if (cn->node.flags & HASHED) {
+ if (snprintf(nn, sizeof(nn), "%s", cn->u.cmd) >= (int)sizeof(nn)) {
+ nn[0] = '\0';
+ }
+ } else {
+ for (pp = path; pp < cn->u.name; pp++) {
if (**pp != '/') {
z = buf;
if (**pp) {
- strucpy(&z, *pp);
+ struncpy(&z, *pp, sizeof(buf) - 1);
*z++ = '/';
+ if ((z - buf) + strlen(arg0) >= sizeof(buf))
+ continue;
}
strcpy(z, arg0);
RET_IF_COM(buf);
}
- strcpy(nn, cn->u.name ? *(cn->u.name) : "");
- strcat(nn, "/");
- strcat(nn, cn->node.nam);
+ }
+ if (snprintf(nn, sizeof(nn), "%s/%s",
+ cn->u.name ? *(cn->u.name) : "",
+ cn->node.nam) >= (int)sizeof(nn))
+ {
+ nn[0] = '\0';
+ }
}
RET_IF_COM(nn);
}
for (pp = path; *pp; pp++) {
z = buf;
if (**pp) {
- strucpy(&z, *pp);
+ struncpy(&z, *pp, sizeof(buf) - 1);
*z++ = '/';
+ if ((z - buf) + strlen(arg0) >= sizeof(buf))
+ continue;
}
strcpy(z, arg0);
RET_IF_COM(buf);