Skip to content

Commit

Permalink
Fix undefined behavior in pty and .sh.match
Browse files Browse the repository at this point in the history
This pull request fixes a few errors that occur when ksh is built
with -fsanitize=address,undefined.

src/cmd/builtin/pty.c:
- Use long typecasts with PROC_op1, which fixes this error under ASan:
  src/cmd/builtin/pty.c:356:18: runtime error: left shift of 12 by 28 places cannot be represented in type 'int'

src/cmd/ksh93/sh/init.c:
- Ensure mp->nodes is not NULL before making use of it. This change
  fixes two errors under ASan that occur during the exit and glob
  regression tests:
  src/cmd/ksh93/sh/init.c:868:6: runtime error: applying non-zero offset 18446744073709551600 to null pointer
  src/cmd/ksh93/sh/init.c:831:5: runtime error: applying non-zero offset 18446744073709551600 to null pointer

src/lib/libast/include/proc.h:
- Remove the now unused PROC_FD_CTTY macro.
  • Loading branch information
JohnoKing committed Jan 23, 2024
1 parent acaac0a commit 7eeafe1
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 31 deletions.
2 changes: 1 addition & 1 deletion src/cmd/builtin/pty.c
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ runcmd(char** argv, int minion, int session)

if (session)
{
ops[0] = PROC_FD_CTTY(minion);
ops[0] = (long)PROC_op1((long)PROC_fd_ctty,(long)minion);
ops[1] = 0;
}
else
Expand Down
64 changes: 35 additions & 29 deletions src/cmd/ksh93/sh/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -828,22 +828,25 @@ static void match2d(struct match *mp)
int i;
Namarr_t *ap;
nv_disc(SH_MATCHNOD, &mp->hdr, NV_POP);
np = nv_namptr(mp->nodes, 0);
for(i=0; i < mp->nmatch; i++)
if(mp->nodes)
{
np->nvname = mp->names + 3 * i;
if(i > 9)
np = nv_namptr(mp->nodes, 0);
for(i=0; i < mp->nmatch; i++)
{
*np->nvname = '0' + i / 10;
np->nvname[1] = '0' + (i % 10);
np->nvname = mp->names + 3 * i;
if(i > 9)
{
*np->nvname = '0' + i / 10;
np->nvname[1] = '0' + (i % 10);
}
else
*np->nvname = '0' + i;
nv_putsub(np, NULL, 1);
nv_putsub(np, NULL, 0);
nv_putsub(SH_MATCHNOD, NULL, i);
nv_arraychild(SH_MATCHNOD, np, 0);
np = nv_namptr(np + 1, 0);
}
else
*np->nvname = '0' + i;
nv_putsub(np, NULL, 1);
nv_putsub(np, NULL, 0);
nv_putsub(SH_MATCHNOD, NULL, i);
nv_arraychild(SH_MATCHNOD, np, 0);
np = nv_namptr(np + 1, 0);
}
if(ap = nv_arrayptr(SH_MATCHNOD))
ap->nelem = mp->nmatch;
Expand All @@ -865,25 +868,28 @@ void sh_setmatch(const char *v, int vsize, int nmatch, int match[], int index)
sh.subshell = 0;
if(index<0)
{
np = nv_namptr(mp->nodes,0);
if(mp->index==0)
match2d(mp);
for(i=0; i < mp->nmatch; i++)
if(mp->nodes)
{
nv_disc(np,&mp->hdr,NV_LAST);
nv_putsub(np,NULL,mp->index);
for(x=mp->index; x >=0; x--)
{
n = i + x*mp->nmatch;
if(mp->match[2*n+1]>mp->match[2*n])
nv_putsub(np,Empty,ARRAY_ADD|x);
}
if((ap=nv_arrayptr(np)) && array_elem(ap)==0)
np = nv_namptr(mp->nodes,0);
if(mp->index==0)
match2d(mp);
for(i=0; i < mp->nmatch; i++)
{
nv_putsub(SH_MATCHNOD,NULL,i);
_nv_unset(SH_MATCHNOD,NV_RDONLY);
nv_disc(np,&mp->hdr,NV_LAST);
nv_putsub(np,NULL,mp->index);
for(x=mp->index; x >=0; x--)
{
n = i + x*mp->nmatch;
if(mp->match[2*n+1]>mp->match[2*n])
nv_putsub(np,Empty,ARRAY_ADD|x);
}
if((ap=nv_arrayptr(np)) && array_elem(ap)==0)
{
nv_putsub(SH_MATCHNOD,NULL,i);
_nv_unset(SH_MATCHNOD,NV_RDONLY);
}
np = nv_namptr(np+1,0);
}
np = nv_namptr(np+1,0);
}
sh.subshell = savesub;
return;
Expand Down
1 change: 0 additions & 1 deletion src/lib/libast/include/proc.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@
#define PROC_op2(o,a,b) (((o)<<(2*PROC_ARG_BIT))|(((b)&PROC_ARG_NULL)<<PROC_ARG_BIT)|((a)&PROC_ARG_NULL))

#define PROC_FD_CLOSE(p,f) PROC_op2(PROC_fd_dup|(f),p,PROC_ARG_NULL)
#define PROC_FD_CTTY(f) PROC_op1(PROC_fd_ctty,f)
#define PROC_FD_DUP(p,c,f) PROC_op2(PROC_fd_dup|(f),p,c)
#define PROC_SIG_DFL(s) PROC_op1(PROC_sig_dfl,s,0)
#define PROC_SIG_IGN(s) PROC_op1(PROC_sig_ign,s,0)
Expand Down

0 comments on commit 7eeafe1

Please sign in to comment.