From 2ae5ce8ab367cc2d0c8eb483228849f22d62cf98 Mon Sep 17 00:00:00 2001 From: copperwater Date: Sun, 21 May 2023 18:35:25 -0400 Subject: [PATCH] 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(). --- doc/lua.adoc | 3 +++ src/sp_lev.c | 39 +++++++++++++++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/doc/lua.adoc b/doc/lua.adoc index a22da3da0..6ac5fd24f 100644 --- a/doc/lua.adoc +++ b/doc/lua.adoc @@ -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: diff --git a/src/sp_lev.c b/src/sp_lev.c index 02e2b7374..3e72d814d 100644 --- a/src/sp_lev.c +++ b/src/sp_lev.c @@ -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;