1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-03-09 15:50:02 +00:00

Fix multiple buffer overflows with justified strings (-L/-R/-Z)

ksh crashed in various different and operating system-dependent
ways when attempting to create or apply justification strings
using typeset -L/-R/-Z, especially if large sizes are used.

The crashes had two immediate causes:
- In nv_newattr(), when applying justification attributes, a buffer
  was allocated for the justified string that was exactly 8 bytes
  longer than the original string. Any larger justification string
  caused a buffer overflow (!!!).
- In nv_putval(), when applying existing attributes to a new value,
  the corresponding memmove() either did not zero-terminate the
  justified string (if the original string was longer than the
  justified string) or could read memory past the original string
  (if the original string was shorter than the justified string).
  Both scenarios can cause a crash.

This commit fixes other minor issues as well, such as a mysterious
8 extra bytes allocated by several malloc/realloc calls. This may
have been some naive attempt to paper over the above bugs. It seems
no one can make any other kind of sense of it.

A readjustment bug with zero-filling was also fixed.

src/cmd/ksh93/sh/name.c:
- nv_putval():
  . Get rid of the magical +8 bytes for malloc and realloc. Just
    allocate one extra byte for the terminating zero.
  . Fix the memmove operation to use strncpy instead, so that
    buffer overflows are avoided in both scenarios described above.
    Also make it conditional upon a size adjustment actually
    happening (i.e. if 'dot' is nonzero).
  . Mild refactoring: combine two 'if(sp)' blocks into one;
    declare variables only used there locally for legibility.
- nv_newattr():
  * Replace the fatally broken "let's allocate string length + 8
    bytes no matter the size of the adjustment" routine with a new
    one based on work by @hyenias (see comments in #142). It is
    efficient with memory, taking into account numeric types,
    growing strings, and shrinking strings.
  * Fix zero-filling in readjustment after changing the initial
    size of a -Z attribute. If the number was zero, all zeros were
    still skipped, leaving an empty string.

Thanks to @hyenias for originally identifying this breakage and
laying the groundwork for fixing nv_newattr(), and to @lijog for
the crash analysis that revealed the key to the nv_putval() fix.

Resolves: https://github.com/ksh93/ksh/issues/142
Resolves: https://github.com/ksh93/ksh/issues/181
This commit is contained in:
Martijn Dekker 2021-02-20 12:27:53 +00:00
parent a959a35291
commit bdb997415d
3 changed files with 70 additions and 25 deletions

6
NEWS
View file

@ -8,6 +8,12 @@ Any uppercase BUG_* names are modernish shell bug IDs.
- Fixed a bug introduced on 2021-01-20: if a DEBUG trap action yielded exit
status 2, the execution of the next command was not skipped as documented.
- Fixed multiple buffer overflows causing crashes in typeset -L/-R-/-Z.
- Fixed typeset -Z zero-filling: if the number was zero, all zeros
were skipped when changing the initial size value of the -Z attribute,
leaving an empty string.
2021-02-18:
- A bug was fixed in the 'read' builtin that caused it to fail to process