From b52edb380ccdd8458872390acbc51202e6d91cd1 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Thu, 21 Apr 2022 02:34:33 +0200 Subject: [PATCH] edit: avoid potential crash with overlapping strings In vi.c, there is a potential use of strcpy(3) on overlapping strings, which is undefined behaviour: > SHOPT_MULTIBYTE == 0 > > # define gencpy(a,b) strcpy((char*)(a),(char*)(b)) > > . > . > . > > if( mode != 'y' ) > { > gencpy(cp,cp+nchars); Thanks to Heiko Berges for the report. src/cmd/ksh93/edit/{edit,emacs,vi}.c: - Change almost all use of strcpy(3) to libast strcopy(), which is a simple implementation that does not have a problem with overlapping strings. Note that the return value is different (it returns a pointer to the terminating '\0') but that is not relevant in any of these cases. - Same for strncpy(3) => libast strncopy(). src/lib/libast/string/strcopy.c: - Backport a couple of cosmetic tweaks from the 93v- beta. --- src/cmd/ksh93/edit/edit.c | 10 +++++----- src/cmd/ksh93/edit/emacs.c | 6 +++--- src/cmd/ksh93/edit/history.c | 2 +- src/cmd/ksh93/edit/vi.c | 12 ++++++------ src/lib/libast/string/strcopy.c | 9 +++++---- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/cmd/ksh93/edit/edit.c b/src/cmd/ksh93/edit/edit.c index 5739a8776..549a146f5 100644 --- a/src/cmd/ksh93/edit/edit.c +++ b/src/cmd/ksh93/edit/edit.c @@ -772,7 +772,7 @@ void ed_setup(register Edit_t *ep, int fd, int reedit) register int shift = 7-ep->e_wsize; ep->e_wsize = 7; pp = ep->e_prompt+1; - strcpy(pp,pp+shift); + strcopy(pp,pp+shift); ep->e_plen -= shift; last[-ep->e_plen-2] = '\r'; } @@ -819,11 +819,11 @@ void ed_setup(register Edit_t *ep, int fd, int reedit) #error no tput method #endif if((pp = nv_getval(SH_SUBSCRNOD)) && strlen(pp) < sizeof(CURSOR_UP)) - strcpy(CURSOR_UP,pp); + strcopy(CURSOR_UP,pp); else CURSOR_UP[0] = '\0'; /* no escape sequence is better than a faulty one */ nv_unset(SH_SUBSCRNOD); - strcpy(ep->e_termname,term); + strcopy(ep->e_termname,term); sh.options = o; sigrelease(SIGINT); } @@ -1493,7 +1493,7 @@ int ed_external(const genchar *src, char *dest) #ifdef _lib_wcscpy wcscpy((wchar_t *)dest,(const wchar_t *)buffer); #else - strcpy(dest,buffer); + strcopy(dest,buffer); #endif return(c); } @@ -1662,7 +1662,7 @@ static int keytrap(Edit_t *ep,char *inbuff,register int insize, int bufsize, int nv_unset(ED_CHRNOD); else if(bufsize>0) { - strncpy(inbuff,cp,bufsize); + strncopy(inbuff,cp,bufsize); inbuff[bufsize-1]='\0'; insize = strlen(inbuff); } diff --git a/src/cmd/ksh93/edit/emacs.c b/src/cmd/ksh93/edit/emacs.c index 954f3462e..1719bf76f 100644 --- a/src/cmd/ksh93/edit/emacs.c +++ b/src/cmd/ksh93/edit/emacs.c @@ -90,8 +90,8 @@ One line screen editor for any program # define digit(c) ((c&~STRIP)==0 && isdigit(c)) #else -# define gencpy(a,b) strcpy((char*)(a),(char*)(b)) -# define genncpy(a,b,n) strncpy((char*)(a),(char*)(b),n) +# define gencpy(a,b) strcopy((char*)(a),(char*)(b)) +# define genncpy(a,b,n) strncopy((char*)(a),(char*)(b),n) # define genlen(str) strlen(str) # define print(c) isprint(c) # define isword(c) (isalnum(out[c]) || (out[c]=='_')) @@ -1425,7 +1425,7 @@ static void search(Emacs_t* ep,genchar *out,int direction) #if SHOPT_MULTIBYTE ed_external(string,(char*)string); #endif /* SHOPT_MULTIBYTE */ - strncpy(lstring,((char*)string)+2,SEARCHSIZE-1); + strncopy(lstring,((char*)string)+2,SEARCHSIZE-1); lstring[SEARCHSIZE-1] = 0; ep->prevdirection = direction; } diff --git a/src/cmd/ksh93/edit/history.c b/src/cmd/ksh93/edit/history.c index 666cd25bc..a9abb9e9b 100644 --- a/src/cmd/ksh93/edit/history.c +++ b/src/cmd/ksh93/edit/history.c @@ -1057,7 +1057,7 @@ char *hist_word(char *string,int size,int word) *cp = 0; if(s1 != string) /* We can't use strcpy() because the two buffers may overlap. */ - memmove(string,s1,strlen(s1)+1); + strcopy(string,s1); return(string); } diff --git a/src/cmd/ksh93/edit/vi.c b/src/cmd/ksh93/edit/vi.c index 00892cfb8..6a6e42a55 100644 --- a/src/cmd/ksh93/edit/vi.c +++ b/src/cmd/ksh93/edit/vi.c @@ -67,8 +67,8 @@ # define isalph(v) _isalph(virtual[v]) #else static genchar _c; -# define gencpy(a,b) strcpy((char*)(a),(char*)(b)) -# define genncpy(a,b,n) strncpy((char*)(a),(char*)(b),n) +# define gencpy(a,b) strcopy((char*)(a),(char*)(b)) +# define genncpy(a,b,n) strncopy((char*)(a),(char*)(b),n) # define genlen(str) strlen(str) # define isalph(v) ((_c=virtual[v])=='_'||isalnum(_c)) # undef isblank @@ -908,7 +908,7 @@ static int cntlmode(Vi_t *vp) hist_copy((char*)virtual, MAXLINE, curhline,-1); else { - strcpy((char*)virtual,(char*)vp->u_space); + strcopy((char*)virtual,(char*)vp->u_space); #if SHOPT_MULTIBYTE ed_internal((char*)vp->u_space,vp->u_space); #endif /* SHOPT_MULTIBYTE */ @@ -1634,7 +1634,7 @@ static int mvcursor(register Vi_t* vp,register int motion) #if SHOPT_MULTIBYTE ed_external(virtual,lsearch+1); #else - strcpy(lsearch+1,virtual); + strcopy(lsearch+1,virtual); #endif /* SHOPT_MULTIBYTE */ *lsearch = '^'; vp->direction = -2; @@ -2362,7 +2362,7 @@ static int search(register Vi_t* vp,register int mode) { /*** user wants repeat of last search ***/ del_line(vp,BAD); - strcpy( ((char*)virtual)+1, lsearch); + strcopy( ((char*)virtual)+1, lsearch); #if SHOPT_MULTIBYTE *((char*)virtual) = '/'; ed_internal((char*)virtual,virtual); @@ -2394,7 +2394,7 @@ static int search(register Vi_t* vp,register int mode) location = hist_find(sh.hist_ptr,((char*)virtual)+1, curhline, 1, new_direction); } cur_virt = i; - strncpy(lsearch, ((char*)virtual)+1, SEARCHSIZE-1); + strncopy(lsearch, ((char*)virtual)+1, SEARCHSIZE-1); lsearch[SEARCHSIZE-1] = 0; if( (curhline=location.hist_command) >=0 ) { diff --git a/src/lib/libast/string/strcopy.c b/src/lib/libast/string/strcopy.c index 5ad7e50c7..62e745d2d 100644 --- a/src/lib/libast/string/strcopy.c +++ b/src/lib/libast/string/strcopy.c @@ -1,8 +1,8 @@ /*********************************************************************** * * * This software is part of the ast package * -* Copyright (c) 1985-2011 AT&T Intellectual Property * -* Copyright (c) 2020-2021 Contributors to ksh 93u+m * +* Copyright (c) 1985-2012 AT&T Intellectual Property * +* Copyright (c) 2020-2022 Contributors to ksh 93u+m * * and is licensed under the * * Eclipse Public License, Version 1.0 * * by AT&T Intellectual Property * @@ -30,7 +30,8 @@ char* strcopy(register char* s, register const char* t) { - if (!t) return(s); + if (!t) + return s; while (*s++ = *t++); - return(--s); + return s - 1; }