Fix: use-after-free when applying an object that got destroyed
Found this running the fuzzer with address sanitizer. After applying an object, the game checks whether it's a talking artifact so that it can speak to you afterwards. If, however, the object got destroyed in the process of applying it, this will read and dereference obj after they have been freed. I found this with a cream pie, which is always destroyed when someone applies it. To fix this, I tracked the obj pointer for what I think are the only two cases in the big switch statement that don't track this - royal jelly and cream pie. Royal jelly is only conditionally destroyed depending on further input, so its function had to be refactored to take a struct obj**, but cream pies are unconditionally destroyed so it can just be set to null.
This commit is contained in:
13
src/apply.c
13
src/apply.c
@@ -33,7 +33,7 @@ static int set_trap(void); /* occupation callback */
|
||||
static void display_polearm_positions(int);
|
||||
static int use_cream_pie(struct obj *);
|
||||
static int jelly_ok(struct obj *);
|
||||
static int use_royal_jelly(struct obj *);
|
||||
static int use_royal_jelly(struct obj **);
|
||||
static int grapple_range(void);
|
||||
static boolean can_grapple_location(coordxy, coordxy);
|
||||
static int use_grapple(struct obj *);
|
||||
@@ -1756,7 +1756,7 @@ dorub(void)
|
||||
if (is_graystone(obj)) {
|
||||
return use_stone(obj);
|
||||
} else if (obj->otyp == LUMP_OF_ROYAL_JELLY) {
|
||||
return use_royal_jelly(obj);
|
||||
return use_royal_jelly(&obj);
|
||||
} else {
|
||||
pline("Sorry, I don't know how to use that.");
|
||||
return ECMD_OK;
|
||||
@@ -3470,10 +3470,11 @@ jelly_ok(struct obj *obj)
|
||||
}
|
||||
|
||||
static int
|
||||
use_royal_jelly(struct obj *obj)
|
||||
use_royal_jelly(struct obj **optr)
|
||||
{
|
||||
int oldcorpsenm;
|
||||
unsigned was_timed;
|
||||
struct obj *obj = *optr;
|
||||
struct obj *eobj;
|
||||
|
||||
if (obj->quan > 1L)
|
||||
@@ -3529,6 +3530,7 @@ use_royal_jelly(struct obj *obj)
|
||||
/* not useup() because we've already done freeinv() */
|
||||
setnotworn(obj);
|
||||
obfree(obj, (struct obj *) 0);
|
||||
*optr = 0;
|
||||
return ECMD_TIME;
|
||||
}
|
||||
|
||||
@@ -4038,9 +4040,10 @@ doapply(void)
|
||||
break;
|
||||
case CREAM_PIE:
|
||||
res = use_cream_pie(obj);
|
||||
obj = (struct obj *) 0;
|
||||
break;
|
||||
case LUMP_OF_ROYAL_JELLY:
|
||||
res = use_royal_jelly(obj);
|
||||
res = use_royal_jelly(&obj);
|
||||
break;
|
||||
case BULLWHIP:
|
||||
res = use_whip(obj);
|
||||
@@ -4187,6 +4190,8 @@ doapply(void)
|
||||
pline("Sorry, I don't know how to use that.");
|
||||
return ECMD_FAIL;
|
||||
}
|
||||
/* This assumes that anything that potentially destroyed obj has kept track
|
||||
* of it and set obj to null before this point. */
|
||||
if (obj && obj->oartifact) {
|
||||
res |= arti_speak(obj); /* sets ECMD_TIME bit if artifact speaks */
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user