doname bounds checking bits
When ready to return, check for overlooked overflow (shouldn't happen) and panic, or report the first excessively long but not overflown description to paniclog, similar to xname. Make ConcUpdate() more robust by not needing bp_eos to be previously set. Less efficient but I think that boat has left the barn? :=} Fix a comment typo.
This commit is contained in:
+36
-12
@@ -89,7 +89,7 @@ struct Jitem {
|
|||||||
ConcUpdate(base); \
|
ConcUpdate(base); \
|
||||||
} while (0)
|
} while (0)
|
||||||
#define ConcUpdate(base) \
|
#define ConcUpdate(base) \
|
||||||
base ## _eos = eos(base ## _eos), \
|
base ## _eos = eos(base), \
|
||||||
/* convert signed ptrdiff_t to unsigned size_t */ \
|
/* convert signed ptrdiff_t to unsigned size_t */ \
|
||||||
base ## spaceleft = (size_t) (base ## _end - base ## _eos)
|
base ## spaceleft = (size_t) (base ## _end - base ## _eos)
|
||||||
|
|
||||||
@@ -573,8 +573,8 @@ xname_flags(
|
|||||||
aside for use by doname() */
|
aside for use by doname() */
|
||||||
buf = gx.xnamep + PREFIX; /* leave room for "17 -3 " */
|
buf = gx.xnamep + PREFIX; /* leave room for "17 -3 " */
|
||||||
buf_end = gx.xnamep + BUFSZ - 1; /* last byte within the obuf[] */
|
buf_end = gx.xnamep + BUFSZ - 1; /* last byte within the obuf[] */
|
||||||
buf[0] = '\0', buf_eos = buf; /* eos(buf) for empty buf[] */
|
buf[0] = '\0';
|
||||||
ConcUpdate(buf); /* update buf_eos (redundantly) and bufspaceleft */
|
ConcUpdate(buf); /* set buf_eos and bufspaceleft */
|
||||||
|
|
||||||
if (Role_if(PM_SAMURAI)) {
|
if (Role_if(PM_SAMURAI)) {
|
||||||
actualn = Japanese_item_name(typ, actualn);
|
actualn = Japanese_item_name(typ, actualn);
|
||||||
@@ -922,8 +922,8 @@ xname_flags(
|
|||||||
it could make buf[] become shorter */
|
it could make buf[] become shorter */
|
||||||
if (pluralize) {
|
if (pluralize) {
|
||||||
obufp = makeplural(buf);
|
obufp = makeplural(buf);
|
||||||
buf[0] = '\0', buf_eos = buf; /* replace the whole string */
|
buf[0] = '\0'; /* replace the whole string */
|
||||||
ConcUpdate(buf);
|
ConcUpdate(buf); /* reset buf_eos and bufspaceleft */
|
||||||
Concat(buf, 0, obufp);
|
Concat(buf, 0, obufp);
|
||||||
releaseobuf(obufp);
|
releaseobuf(obufp);
|
||||||
}
|
}
|
||||||
@@ -1345,7 +1345,7 @@ doname_base(
|
|||||||
: doffing(obj) ? " (being doffed)"
|
: doffing(obj) ? " (being doffed)"
|
||||||
: donning(obj) ? " (being donned)"
|
: donning(obj) ? " (being donned)"
|
||||||
: " (being worn)");
|
: " (being worn)");
|
||||||
/* we just added a parenthesized pharse, but the right paren
|
/* we just added a parenthesized phrase, but the right paren
|
||||||
might be absent if the appended string got truncated */
|
might be absent if the appended string got truncated */
|
||||||
if (bp_eos[-1] == ')') {
|
if (bp_eos[-1] == ')') {
|
||||||
/* slippery fingers is an intrinsic condition of the hero
|
/* slippery fingers is an intrinsic condition of the hero
|
||||||
@@ -1631,7 +1631,9 @@ doname_base(
|
|||||||
else
|
else
|
||||||
ConcatF1(bp, 0, " (%u aum)", obj->owt);
|
ConcatF1(bp, 0, " (%u aum)", obj->owt);
|
||||||
|
|
||||||
/* ConcatF1(bp) updates bp_eos + bpspaceleft; we're done with them */
|
/* ConcatF1(bp) updates bp_eos and bpspaceleft but we're done
|
||||||
|
with them now; add a fake use so compiler won't complain
|
||||||
|
about a variable assignment that won't be subsequently used */
|
||||||
nhUse(bp_eos);
|
nhUse(bp_eos);
|
||||||
nhUse(bpspaceleft);
|
nhUse(bpspaceleft);
|
||||||
}
|
}
|
||||||
@@ -1639,19 +1641,41 @@ doname_base(
|
|||||||
bp = strprepend(bp, prefix);
|
bp = strprepend(bp, prefix);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
* Last gasp bounds check.
|
||||||
|
*
|
||||||
* If caller intends this to be for a menu entry, make sure that
|
* If caller intends this to be for a menu entry, make sure that
|
||||||
* there is some room to combine with menu selector prefix without
|
* there is some room to combine with menu selector prefix without
|
||||||
* exceeding BUFSZ-1.
|
* exceeding BUFSZ-1.
|
||||||
*
|
*
|
||||||
* 4: width of menu entry selector text: "c - " for tty. For
|
* offsetbp=4: width of menu entry selector text: "c - " for tty.
|
||||||
* curses, that wastes a space since it only needs 3: "c) ".
|
* For curses, that wastes a char since it only needs 3: "c) ".
|
||||||
*
|
*
|
||||||
* Reaching full BUFSZ-1 length can't happen unless both doname
|
* Reaching full BUFSZ-1 length can't happen unless both doname
|
||||||
* (BUFSZ-PREFIX) and strprepend (PREFIX) use up all available
|
* (BUFSZ-PREFIX) and strprepend (PREFIX) use up all available
|
||||||
* space.
|
* space or one of them overflows without being detected.
|
||||||
*/
|
*/
|
||||||
if (for_menu && strlen(bp) + 4 > BUFSZ - 1)
|
if (strlen(bp) > BUFSZ - 1) {
|
||||||
bp[BUFSZ - 1 - 4] = '\0';
|
paniclog("doname", bp);
|
||||||
|
/* ideally this will never happen; if xnamep is any obuf[]
|
||||||
|
other than the last, overflow here would be relatively
|
||||||
|
benign and we could probably keep going */
|
||||||
|
panic("doname: long object description overflow.");
|
||||||
|
/*NOTREACHED*/
|
||||||
|
} else {
|
||||||
|
static int doname_full = 0;
|
||||||
|
int offsetbp = for_menu ? 4 : 0;
|
||||||
|
|
||||||
|
if (strlen(bp) + offsetbp >= BUFSZ - 1) {
|
||||||
|
/* for !offsetbp, we'll only get here if strlen(bp)==BUFSZ-1 */
|
||||||
|
if (!doname_full++) {
|
||||||
|
paniclog("doname", bp);
|
||||||
|
Sprintf(tmpbuf, "long object description%s.",
|
||||||
|
offsetbp ? " truncated for menu use" : "");
|
||||||
|
paniclog("doname", tmpbuf);
|
||||||
|
}
|
||||||
|
bp[BUFSZ - 1 - offsetbp] = '\0';
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return bp;
|
return bp;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user