1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-13 03:32:24 +00:00

Fix various crashes by removing invalid memccpy() use

The sfputr() function (put out a null-terminated string) contained
a use of memccpy() that was invalid and could cause crashes,
because the buffer it was copying into could overlap or even be
identical with the buffer being copied from.

Among (probably) other things, this commit fixes a crash in 'print
-v' (print a compound variable structure) on macOS, that caused the
comvar.sh and comvario.sh regression tests to fail spectacularly.
Now they pass.

Issue discovered and fixed by Kurtis Rader in the abandoned ksh2020
branch; this commit backports the fix. He wrote:

| #if _lib_memccpy && !__ia64 /* these guys may never get it right */
|
| The problem is that assertion is wrong. It implies that the libc
| implementation of memccpy() on IA64 is broken. Which is
| incorrect. The problem is the AST sfputr() function is depending
| on what is explicitly undefined behavior in the face of
| overlapping source and destination buffers.
| [...] Using memccpy() simply complicates the code and is unlikely
| to be measurably, let alone noticeably, faster.

Further discussion/analysis: https://github.com/att/ast/issues/78

src/lib/libast/sfio/sfputr.c:
- Remove memccpy use. Always use the manual copying loop.

(cherry picked from commit fbe3c83335256cb714a4aa21f555083c9f1d71d8)
This commit is contained in:
Martijn Dekker 2020-06-05 16:03:07 +02:00
parent 6f0e008cf7
commit 8b07d2a011
2 changed files with 10 additions and 7 deletions

4
NEWS
View file

@ -11,6 +11,10 @@ Any uppercase BUG_* names are modernish shell bug IDs.
(unset PATH; PATH=/dev/null; ls); : wrongly ran 'ls'
(unset LC_ALL; LC_ALL=badlocale); : failed to print a diagnostic
- Fix crashes on some systems, including at least a crash in 'print -v' on
macOS, by eliminating an invalid/undefined use of memccpy() on overlapping
buffers in the commonly used sfputr() function.
2020-06-04:
- Fix BUG_KBGPID: the $! special parameter was not set if a background job

View file

@ -105,16 +105,15 @@ int rc; /* record separator. */
break;
}
#if _lib_memccpy && !__ia64 /* these guys may never get it right */
if((ps = (uchar*)memccpy(ps,s,'\0',p)) != NIL(uchar*))
ps -= 1;
else ps = f->next+p;
s += ps - f->next;
#else
/*
* Do not replace the following loop with memccpy(). The
* 'ps' and 's' buffers may overlap or even point to the
* same buffer. See: https://github.com/att/ast/issues/78
*/
for(; p > 0; --p, ++ps, ++s)
if((*ps = *s) == 0)
break;
#endif
w += ps - f->next;
f->next = ps;
}