hypothetical buffer overruns

doprtool() and doprinuse() collect the inventory letters of all
applicable items into a buffer capable of holding 52 letters plus
terminator.  It is possible to have more than 52 items (ignoring
gold) so theoretically possible to have more than 52 separate lit
candles.  Guard against that.

The easiest way to get an item in the overflow slot is to carry
52 non-boulders, polymorph into a giant, and pick up a boulder.
Assigning the latter to one of the three weapon slots would not
impact doprtool() but it will impact doprinuse().  However, that
wasn't enough to cause a crash for me; evidently the overflow
clobbered something innocuous.  (52+boulder is not the only way
to get something into slot '#', just the only guaranteed one I
can think of offhand.)

This also removes a bunch of 'register' type qualifiers.

diff --git a/doc/fixes37.0 b/doc/fixes37.0
index 7f22366e99f032854777144fa8dc006476f9b9f6..914e32cceaabd4e1542f93a759ab6bc6fbfe112a 100644 (file)
--- a/doc/fixes37.0
+++ b/doc/fixes37.0
@@ -1,4 +1,4 @@
-NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.463 $ $NHDT-Date: 1614291055 2021/02/25 22:10:55 $
+NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.465 $ $NHDT-Date: 1614474790 2021/02/28 01:13:10 $
 
 General Fixes and Modified Features
 -----------------------------------
@@ -389,6 +389,9 @@ messages when Minetown watchmen become angry could report "you see an angry
        guard approaching" even if he was invisible and hero can't see invis
 when autopickup is on but disabled due to being inside a shop, have ^X say so
 don't force fake player monks to always be male
+it was theoretically possible to overflow an internal buffer containing
+       inventory letters by carrying more than 52 separate lit candles and
+       using the '(' or '*' commands
 
 
 Fixes to 3.7.0-x Problems that Were Exposed Via git Repository

diff --git a/src/invent.c b/src/invent.c
index 0a4d178ddf292c19beadef44fc1a2adbd346e47c..2da82e7a390f3664afdc4bf372fba2a5a5b2b14e 100644 (file)
--- a/src/invent.c
+++ b/src/invent.c
@@ -1,4 +1,4 @@
-/* NetHack 3.7 invent.c        $NHDT-Date: 1612912018 2021/02/09 23:06:58 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.319 $ */
+/* NetHack 3.7 invent.c        $NHDT-Date: 1614474790 2021/02/28 01:13:10 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.320 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /*-Copyright (c) Derek S. Ray, 2015. */
 /* NetHack may be freely redistributed.  See license for details. */
@@ -570,7 +570,7 @@ sortloot(struct obj **olist, unsigned mode, /* flags for sortloot_cmp() */
 #endif /*0*/
 
 void
-assigninvlet(register struct obj *otmp)
+assigninvlet(struct obj *otmp)
 {
     boolean inuse[52];
     register int i;
@@ -1079,7 +1079,7 @@ useupall(struct obj *obj)
 }
 
 void
-useup(register struct obj *obj)
+useup(struct obj *obj)
 {
     /* Note:  This works correctly for containers because they (containers)
        don't merge. */
@@ -1155,7 +1155,7 @@ freeinv_core(struct obj *obj)
 
 /* remove an object from the hero's inventory */
 void
-freeinv(register struct obj *obj)
+freeinv(struct obj *obj)
 {
     extract_nobj(obj, &g.invent);
     freeinv_core(obj);
@@ -1180,7 +1180,7 @@ delallobj(int x, int y)
 
 /* destroy object in fobj chain (if unpaid, it remains on the bill) */
 void
-delobj(register struct obj *obj)
+delobj(struct obj *obj)
 {
     boolean update_map;
 
@@ -1234,7 +1234,7 @@ nxtobj(struct obj *obj, int type, boolean by_nexthere)
 }
 
 struct obj *
-carrying(register int type)
+carrying(int type)
 {
     register struct obj *otmp;
 
@@ -1323,7 +1323,7 @@ u_have_novel(void)
 }
 
 struct obj *
-o_on(unsigned int id, register struct obj *objchn)
+o_on(unsigned int id, struct obj *objchn)
 {
     struct obj *temp;
 
@@ -1338,7 +1338,7 @@ o_on(unsigned int id, register struct obj *objchn)
 }
 
 boolean
-obj_here(register struct obj *obj, int x, int y)
+obj_here(struct obj *obj, int x, int y)
 {
     register struct obj *otmp;
 
@@ -1349,7 +1349,7 @@ obj_here(register struct obj *obj, int x, int y)
 }
 
 struct obj *
-g_at(register int x, register int y)
+g_at(int x, int y)
 {
     register struct obj *obj = g.level.objects[x][y];
 
@@ -1363,7 +1363,7 @@ g_at(register int x, register int y)
 
 /* compact a string of inventory letters by dashing runs of letters */
 static void
-compactify(register char *buf)
+compactify(char *buf)
 {
     register int i1 = 1, i2 = 1;
     register char ilet, ilet1, ilet2;
@@ -1457,7 +1457,7 @@ any_obj_ok(struct obj *obj)
  * it with &cg.zeroobj, so its behavior can be undefined in that case.
  */
 struct obj *
-getobj(register const char *word,
+getobj(const char *word,
        int (*obj_ok)(OBJ_P), /* callback */
        unsigned int ctrlflags)
 {
@@ -1982,7 +1982,7 @@ askchain(struct obj **objchn, /* *objchn might change */
          int mx, const char *word)
 {
     struct obj *otmp, *otmpo;
-    register char sym, ilet;
+    char sym, ilet;
     int cnt = 0, dud = 0, tmp;
     boolean takeoff, nodot, ident, take_out, put_in, first, ininv, bycat;
     char qbuf[QBUFSZ], qpfx[QBUFSZ];
@@ -2874,7 +2874,7 @@ dounpaid(void)
 {
     winid win;
     struct obj *otmp, *marker, *contnr;
-    register char ilet;
+    char ilet;
     char *invlet = flags.inv_order;
     int classcount, count, num_so_far;
     long cost, totcost;
@@ -3644,7 +3644,7 @@ int
 doprarm(void)
 {
     char lets[8];
-    register int ct = 0;
+    int ct = 0;
     /*
      * Note:  players sometimes get here by pressing a function key which
      * transmits ''ESC [ <something>'' rather than by pressing '[';
@@ -3678,11 +3678,11 @@ doprarm(void)
 int
 doprring(void)
 {
-    if (!uleft && !uright)
+    if (!uleft && !uright) {
         You("are not wearing any rings.");
-    else {
+    } else {
         char lets[3];
-        register int ct = 0;
+        int ct = 0;
 
         if (uleft)
             lets[ct++] = obj_to_let(uleft);
@@ -3725,8 +3725,13 @@ doprtool(void)
     char lets[52 + 1];
 
     for (otmp = g.invent; otmp; otmp = otmp->nobj)
-        if (tool_in_use(otmp))
+        if (tool_in_use(otmp)) {
+            /* we could be carrying more than 52 items; theoretically they
+               might all be lit candles so avoid potential lets[] overflow */
+            if (ct >= (int) sizeof lets - 1)
+                break;
             lets[ct++] = obj_to_let(otmp);
+        }
     lets[ct] = '\0';
     if (!ct)
         You("are not using any tools.");
@@ -3745,8 +3750,13 @@ doprinuse(void)
     char lets[52 + 1];
 
     for (otmp = g.invent; otmp; otmp = otmp->nobj)
-        if (is_worn(otmp) || tool_in_use(otmp))
+        if (is_worn(otmp) || tool_in_use(otmp)) {
+            /* we could be carrying more than 52 items; theoretically they
+               might all be lit candles so avoid potential lets[] overflow */
+            if (ct >= (int) sizeof lets - 1)
+                break;
             lets[ct++] = obj_to_let(otmp);
+        }
     lets[ct] = '\0';
     if (!ct)
         You("are not wearing or wielding anything.");
@@ -3759,9 +3769,9 @@ doprinuse(void)
  * uses up an object that's on the floor, charging for it as necessary
  */
 void
-useupf(register struct obj *obj, long numused)
+useupf(struct obj *obj, long numused)
 {
-    register struct obj *otmp;
+    struct obj *otmp;
     boolean at_u = (obj->ox == u.ux && obj->oy == u.uy);
 
     /* burn_floor_objects() keeps an object pointer that it tries to
@@ -4228,7 +4238,7 @@ worn_wield_only(struct obj *obj)
  *      MINV_ALL            - display all inventory
  */
 struct obj *
-display_minventory(register struct monst *mon, int dflags, char *title)
+display_minventory(struct monst *mon, int dflags, char *title)
 {
     struct obj *ret;
     char tmp[QBUFSZ];
@@ -4276,7 +4286,7 @@ display_minventory(register struct monst *mon, int dflags, char *title)
  * Currently, this is only used for statues, via wand of probing.
  */
 struct obj *
-display_cinventory(register struct obj *obj)
+display_cinventory(struct obj *obj)
 {
     struct obj *ret;
     char qbuf[QBUFSZ];