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

Fix crashes on redefining/unsetting predefined alias (re: 7e7f1372)

Reproducer 1:

  $ alias r
  r='hist -s'
  $ alias r=foo
  $ unalias r
  ksh(10127,0x10d6c35c0) malloc: *** error for object 0x7ffdcd404468: pointer being freed was not allocated
  ksh(10127,0x10d6c35c0) malloc: *** set a breakpoint in malloc_error_break to debug
  Abort

The crash happens as unall() (typeset.c) calls nv_delete() (name.c)
which tries to free a node pointer that was not directly allocated.

Reproducer 2:

  $ ENV=/./dev/null ksh
  $ echo : >script
  $ chmod +x script
  $ alias r=foo
  $ ./script
  ksh(10193,0x10c8505c0) malloc: *** error for object 0x7fa136c04468: pointer being freed was not allocated
  ksh(10193,0x10c8505c0) malloc: *** set a breakpoint in malloc_error_break to debug
  Abort

This crash happens for the same reason, but in another location,
namely in sh_reinit() (init.c) as it is freeing up the alias table
before executing a script that does not start with a #! path.

This is a serious bug because it is not uncommon for .kshrc or
.profile scripts to (re)define an alias called 'r'.

Analysis:

These crashes happen because the incorrectly freed node pointer is
part of a larger block of nodes initialised by sh_inittree() in
init.c. That function allocates all the nodes for a table (see
data/{aliases,builtins,variables}.c) in a contiguous block
addressable by numeric index (see builtins.h and variables.h for
how that is used).

So, while the value of the alias is correctly marked NV_NOFREE and
is not freed, that attribute does not apply to the node pointer
itself, which also is not freeable. Thus, if the value is replaced
by a freeable one, the node pointer is incorrectly freed upon
unaliasing it, and the shell crashes.

The simplest fix is to allocate each predefined alias node
individually, like any other alias -- because, in fact, we do not
need the predefined alias nodes to be in a contiguous addressable
block; there is nothing that specifically addresses these aliases.

src/cmd/ksh93/sh/main.c: sh_main():
- Instead of calling sh_inittree(), use a dedicated loop to
  allocate each predefined alias node individually, making each
  node invidually freeable. The value does remain non-freeable,
  but the NV_NOFREE attribute correctly takes care of that.

src/cmd/ksh93/bltins/typeset.c:
- Get rid of the incomplete and now unnecessary workarounds in
  unall() and sh_reinit().

Thanks to @jghub and @JohnoKing for finding and reporting the bug.
Discussion: https://github.com/ksh93/ksh/discussions/503#discussioncomment-3337172
This commit is contained in:
Martijn Dekker 2022-08-06 10:47:24 +01:00
parent 9c9743998f
commit cd0638690c
7 changed files with 40 additions and 21 deletions

View file

@ -272,7 +272,8 @@ The calls \f3dtinsert()\fP and \f3dtinstall()\fP will insert a new object
in front of such a current object
while the call \f3dtappend()\fP will append in back of it.
.Ss " Dtdeque"
Objects are kept in a deque. This is similar to \f3Dtlist\fP
Objects are kept in a deque (double-ended queue; pronounce "deck").
This is similar to \f3Dtlist\fP
except that objects are always inserted at the front and appended at the tail
of the list.
.Ss " Dtstack"
@ -467,10 +468,10 @@ See \f3Dtdisc_t.makef\fP for object construction.
\f3dtinsert()\fP and \f3dtappend()\fP perform the same function
for all methods except for \f3Dtlist\fP (see \f3Dtlist\fP for details).
For \f3Dtset\fP, \f3Dtrhset\fP or \f3Dtoset\fP,
if there is an object in \f3dt\fP matching \f3obj\fP
\f3dtinsert()\fP and \f3dtappend()\fP will not insert a new object.
On the other hand, \f3dtinstall()\fP remove such a matching
object then insert the new object.
if there is an object in \f3dt\fP matching \f3obj\fP,
\f3dtinsert()\fP and \f3dtappend()\fP will not insert a new object,
whereas \f3dtinstall()\fP will remove such a matching
object before inserting the new object.
On failure, \f3dtinsert()\fP and \f3dtinstall()\fP return \f3NULL\fP.
Otherwise, the return value is either the newly inserted object