external/cde
1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-15 04:32:24 +00:00
Code Issues Releases Wiki Activity

Fix use after free in sh_funstaks() (re: 69d37d5e)

Browse source 
The referenced commit introduced the NIL (NULL) assignment in:

	stakdelete(slpold->slptr);
	slpold->slptr = NIL(Stak_t*);

First the stack is closed/freed with stakdelete() a.k.a.
stkclose(), then its pointer is reset. Looks correct, right?

Wrong: slpold may itself be in the allocated region that
slpold->slptr points to. That's because we're dealing with a linked
list of stacks, in which a pointer on each stack points to the next
stack. So there are scenarios in which, after the stakdelete()
call, dereferencing slpold is a use after free.

Most systems quietly tolerate this use after free. But, according
to @JohnoKing's testing, this bug was causing 23 crashes in the
regression tests after compiling ksh with AddressSanitizer enabled.

src/cmd/ksh93/sh/parse.c: sh_funstaks():
- Save the value of slpold->slptr and reset that pointer before
  calling stakdelete() a.k.a. stkclose().

Resolves: https://github.com/ksh93/ksh/issues/517
This commit is contained in:
Martijn Dekker 2022-08-19 15:25:40 +01:00
parent 274331acac
commit f24040ee45
3 changed files with 11 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/cmd/ksh93/include/argnod.h
View file

@ -53,7 +53,7 @@ struct comnod
#define COMSCAN (01<<COMBITS) #define COMSCAN (01<<COMBITS)
#define COMFIXED (02<<COMBITS) #define COMFIXED (02<<COMBITS)
struct slnod /* struct for link list of stacks */ struct slnod /* struct for linked list of stacks */
{ {
struct slnod *slnext; struct slnod *slnext;
struct slnod *slchild; struct slnod *slchild;

2
src/cmd/ksh93/include/version.h
View file

@ -18,7 +18,7 @@
#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
#define SH_RELEASE_SVER "1.0.3-alpha" /* semantic version number: https://semver.org */ #define SH_RELEASE_SVER "1.0.3-alpha" /* semantic version number: https://semver.org */
#define SH_RELEASE_DATE "2022-08-18" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_DATE "2022-08-19" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_CPYR "(c) 2020-2022 Contributors to ksh " SH_RELEASE_FORK #define SH_RELEASE_CPYR "(c) 2020-2022 Contributors to ksh " SH_RELEASE_FORK
/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */ /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */

10
src/cmd/ksh93/sh/parse.c
View file

@ -528,8 +528,16 @@ void sh_funstaks(register struct slnod *slp,int flag)
{ {
if(flag<=0) if(flag<=0)
{ {
stakdelete(slpold->slptr); /*
* Since we're dealing with a linked list of stacks, slpold may be inside the allocated region
* pointed to by slpold->slptr, meaning the stakdelete() call may invalidate slpold as well as
* slpold->slptr. So if we do 'stakdelete(slpold->slptr); slpold->slptr = NIL(Stak_t*)' as may
* seem obvious, the assignment may be a use-after-free of slpold. Therefore, save the pointer
* value and reset the pointer before closing/freeing the stack.
*/
Stak_t *sp = slpold->slptr;
slpold->slptr = NIL(Stak_t*); slpold->slptr = NIL(Stak_t*);
stakdelete(sp);
} }
else else
staklink(slpold->slptr); staklink(slpold->slptr);