Fix and guard against out-of-bounds writes in splev code

I traced a memory corruption bug in xNetHack to a themed room that
looked something like this:

    function()
       des.room({ type="themed", contents = function()
          des.feature({ type='sink' })
          ...
       end })
    end

Placing a feature at a random spot within a room or region is a
reasonable thing for the parser to handle, but the code was not equipped
to handle it, and so the unspecified x and y set as -1 got passed
directly to SP_COORD_PACK, ending up as coordinates way off the map.
Since sel_set_feature does not do an isok() check, this ended up writing
data to unrelated memory.

This commit does the following things:

- Enables des.feature() with no coordinates specified, both via a table
  with 'type' set, and as the single string argument. When no
  coordinates are specified, it will pick a random normal-floor spot
  within the enclosing room or region if there is one, or anywhere
  on the level if there isn't.
- Prevents sel_set_feature from corrupting memory outside
  g.level.locations. Additionally, if EXTRA_SANITY_CHECKS is defined and
  this gets attempted, it causes an impossible.
- Guards the existing "door coord not ok" Lua error with an immediate
  return from lspo_door.
- Adds similar "coord not ok" errors to all the other locations in
  sp_lev.c which did not already check for a unspecified/invalid
  coordinate and for which a random coordinate is nonsensical:
  des.terrain(), des.drawbridge(), and des.mazewalk().
This commit is contained in:
copperwater
2023-05-21 18:35:25 -04:00
committed by PatR
parent c016f70a0d
commit 2ae5ce8ab3
2 changed files with 38 additions and 4 deletions

View File

@@ -534,6 +534,9 @@ Create a feature, and set flags for it.
Valid features are a fountain, a sink, a pool, a throne, or a tree.
Throne has `looted` flag, tree has `looted` and `swarm`, fountain has `looted` and `warned`,
sink has `pudding`, `dishwasher`, and `ring`.
If passed with no coordinates, it will be placed in a random normal-floor spot
in the enclosing room or region if one exists, or a random normal-floor spot
anywhere on the level if one does not exist.
Example:

View File

@@ -5256,6 +5256,12 @@ sel_set_ter(coordxy x, coordxy y, genericptr_t arg)
static void
sel_set_feature(coordxy x, coordxy y, genericptr_t arg)
{
if (!isok(x, y)) {
#ifdef EXTRA_SANITY_CHECKS
impossible("sel_set_feature(%i,%i,%i) !isok", x, y, (*(int *) arg));
#endif /*EXTRA_SANITY_CHECKS*/
return;
}
if (IS_FURNITURE(levl[x][y].typ))
return;
levl[x][y].typ = (*(int *) arg);
@@ -5337,8 +5343,10 @@ lspo_door(lua_State *L)
/*selection_iterate(sel, sel_set_door, (genericptr_t) &typ);*/
get_location_coord(&x, &y, ANY_LOC, gc.coder->croom,
SP_COORD_PACK(x, y));
if (!isok(x, y))
if (!isok(x, y)) {
nhl_error(L, "door coord not ok");
return 0;
}
sel_set_door(x, y, (genericptr_t) &typ);
}
@@ -5461,10 +5469,15 @@ lspo_feature(lua_State *L)
int typ;
int argc = lua_gettop(L);
boolean can_have_flags = FALSE;
long fcoord;
int humidity;
create_des_coder();
if (argc == 2 && lua_type(L, 1) == LUA_TSTRING
if (argc == 1 && lua_type(L, 1) == LUA_TSTRING) {
typ = features2i[luaL_checkoption(L, 1, NULL, features)];
x = y = -1;
} else if (argc == 2 && lua_type(L, 1) == LUA_TSTRING
&& lua_type(L, 2) == LUA_TTABLE) {
lua_Integer fx, fy;
typ = features2i[luaL_checkoption(L, 1, NULL, features)];
@@ -5485,7 +5498,15 @@ lspo_feature(lua_State *L)
can_have_flags = TRUE;
}
get_location_coord(&x, &y, ANY_LOC, gc.coder->croom, SP_COORD_PACK(x, y));
if (x == -1 && y == -1) {
fcoord = SP_COORD_PACK_RANDOM(0);
humidity = DRY; /* pick a regular space, no rock or other furniture */
}
else {
fcoord = SP_COORD_PACK(x, y);
humidity = ANY_LOC; /* assume the author knows what they're doing */
}
get_location_coord(&x, &y, humidity, gc.coder->croom, fcoord);
if (typ == STONE)
impossible("feature has unknown type param.");
@@ -5582,6 +5603,10 @@ lspo_terrain(lua_State *L)
} else {
get_location_coord(&x, &y, ANY_LOC, gc.coder->croom,
SP_COORD_PACK(x, y));
if (!isok(x, y)) {
nhl_error(L, "terrain coord not ok");
return 0;
}
sel_set_ter(x, y, (genericptr_t) &tmpterrain);
}
@@ -6251,6 +6276,10 @@ lspo_drawbridge(lua_State *L)
y = my;
get_location_coord(&x, &y, DRY | WET | HOT, gc.coder->croom, dcoord);
if (!isok(mx, my)) {
nhl_error(L, "drawbridge coord not ok");
return 0;
}
if (db_open == -1)
db_open = !rn2(2);
if (!create_drawbridge(x, y, dir, db_open ? TRUE : FALSE))
@@ -6300,8 +6329,10 @@ lspo_mazewalk(lua_State *L)
get_location_coord(&x, &y, ANY_LOC, gc.coder->croom, mcoord);
if (!isok(x, y))
if (!isok(x, y)) {
nhl_error(L, "mazewalk coord not ok");
return 0;
}
if (ftyp < 1) {
ftyp = gl.level.flags.corrmaze ? CORR : ROOM;