nethack-gwaa-kiwi/nethack
1
0
Fork 0
Code Pull Requests Activity

fix PR #1254 - avoid signed integer overflow

Browse Source 
Pull request from mkuoppal:  avoid integer overflow when user types
digits and they're combined into a number by successively multiplying
intermediate value by 10 and adding new digit.  Needed to avoid
triggering undefined behavior if the value overflows the largest
signed integer (actually long int).

This is a much more general fix than the code in the pull request,
which imposed an arbitrary limit for one aspect of tty input.

I'm not convinced that integer.h was the right place to add the new
AppendLongDigit() macro.  I may not have caught all the places where
it is needed.  files.c accumulates a value from digits but uses
unsigned int, so overflow won't trigger undefined behavior (although
it presumably ends up with a different value than what was intended).
options.c and coloratt.c accumulate smaller integers and have a limit
on the number of digits they'll use, so can't overflow.

Fixes #1254
This commit is contained in:
PatR
2024-06-09 14:17:14 -07:00
parent 50e9b7f404
commit 05cbbc7181
6 changed files with 44 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
include/integer.h
View File

@@ -1,4 +1,4 @@
/* NetHack 3.7 integer.h $NHDT-Date: 1596498539 2020/08/03 23:48:59 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.8 $ */
/* NetHack 3.7 integer.h $NHDT-Date: 1717967331 2024/06/09 21:08:51 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.12 $ */
/* Copyright (c) 2016 by Michael Allison */
/* NetHack may be freely redistributed. See license for details. */
@@ -101,4 +101,13 @@ typedef uint32_t uint32;
typedef int64_t int64;
typedef uint64_t uint64;
/* for non-negative L, calculate L * 10 + D, avoiding signed overflow;
yields -1 if overflow would have happened;
assumes compiler will optimize the constants */
#define AppendLongDigit(L,D) \
(((L) < LONG_MAX / 10L \
|| ((L) == LONG_MAX / 10L && (D) <= LONG_MAX % 10L)) \
? (L) * 10L + (D) \
: -1L)
#endif /* INTEGER_H */

7
src/cmd.c
View File

@@ -1,4 +1,4 @@
/* NetHack 3.7 cmd.c $NHDT-Date: 1716592962 2024/05/24 23:22:42 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.728 $ */
/* NetHack 3.7 cmd.c $NHDT-Date: 1717967336 2024/06/09 21:08:56 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.729 $ */
/* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/*-Copyright (c) Robert Patrick Rankin, 2013. */
/* NetHack may be freely redistributed. See license for details. */
@@ -4669,7 +4669,10 @@ get_count(
}
if (digit(key)) {
cnt = 10L * cnt + (long) (key - '0');
long dgt = (long) (key - '0');
/* cnt = (10 * cnt) + (key - '0'); */
cnt = AppendLongDigit(cnt, dgt);
if (cnt < 0L)
cnt = 0L;
else if (maxcount > 0L && cnt > maxcount)

11
win/X11/winX.c
View File

@@ -1,4 +1,4 @@
/* NetHack 3.7 winX.c $NHDT-Date: 1643491577 2022/01/29 21:26:17 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.110 $ */
/* NetHack 3.7 winX.c $NHDT-Date: 1717967337 2024/06/09 21:08:57 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.136 $ */
/* Copyright (c) Dean Luick, 1992 */
/* NetHack may be freely redistributed. See license for details. */
@@ -2219,8 +2219,15 @@ yn_key(Widget w, XEvent *event, String *params, Cardinal *num_params)
} else {
if (yn_getting_num) {
if (digit(ch)) {
long dgt = (long) (ch - '0');
yn_ndigits++;
yn_val = (yn_val * 10) + (long) (ch - '0');
/* yn_val = (10 * yn_val) + (ch - '0'); */
yn_val = AppendLongDigit(yn_val, dgt);
if (yn_val < 0L) {
yn_ndigits = 0;
yn_val = 0;
}
return; /* wait for more input */
}
if (yn_ndigits && (ch == '\b' || ch == 127 /*DEL*/)) {

7
win/tty/topl.c
View File

@@ -1,4 +1,4 @@
/* NetHack 3.7 topl.c $NHDT-Date: 1596498344 2020/08/03 23:45:44 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.72 $ */
/* NetHack 3.7 topl.c $NHDT-Date: 1717967339 2024/06/09 21:08:59 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.89 $ */
/* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/*-Copyright (c) Michael Allison, 2009. */
/* NetHack may be freely redistributed. See license for details. */
@@ -493,7 +493,10 @@ tty_yn_function(
if (!preserve_case)
z = lowc(z);
if (digit(z)) {
value = (10 * value) + (z - '0');
long dgt = (long) (z - '0');
/* value = (10 * value) + (z - '0'); */
value = AppendLongDigit(value, dgt);
if (value < 0)
break; /* overflow: try again */
digit_string[0] = z;

11
win/tty/wintty.c
View File

@@ -1,4 +1,4 @@
/* NetHack 3.7 wintty.c $NHDT-Date: 1717482166 2024/06/04 06:22:46 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.408 $ */
/* NetHack 3.7 wintty.c $NHDT-Date: 1717967340 2024/06/09 21:09:00 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.410 $ */
/* Copyright (c) David Cohrs, 1991 */
/* NetHack may be freely redistributed. See license for details. */
@@ -1566,7 +1566,14 @@ process_menu_window(winid window, struct WinDesc *cw)
if (!counting && strchr(gacc, morc))
goto group_accel;
count = (count * 10L) + (long) (morc - '0');
{
long dgt = (long) (morc - '0');
/* count = (10 * count) + (morc - '0'); */
count = AppendLongDigit(count, dgt);
if (count < 0L) /* overflow */
continue; /* reset_count is True */
}
/*
* It is debatable whether we should allow 0 to
* start a count. There is no difference if the

8
win/win32/mswproc.c
View File

@@ -1,4 +1,4 @@
/* NetHack 3.7 mswproc.c $NHDT-Date: 1613292828 2021/02/14 08:53:48 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.165 $ */
/* NetHack 3.7 mswproc.c $NHDT-Date: 1717967341 2024/06/09 21:09:01 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.193 $ */
/* Copyright (C) 2001 by Alex Kompel */
/* NetHack may be freely redistributed. See license for details. */
@@ -1637,6 +1637,7 @@ mswin_yn_function(const char *question, const char *choices, char def)
char z, digit_string[2];
int n_len = 0;
long value = 0;
mswin_putstr_ex(WIN_MESSAGE, ATR_BOLD, ("#"), 1);
n_len++;
digit_string[1] = '\0';
@@ -1650,7 +1651,10 @@ mswin_yn_function(const char *question, const char *choices, char def)
do { /* loop until we get a non-digit */
z = lowc(readchar());
if (digit(z)) {
value = (10 * value) + (z - '0');
long dgt = (long) (z - '0');
/* value = (10 * value) + (z - '0'); */
value = AppendLongDigit(value, dgt);
if (value < 0)
break; /* overflow: try again */
digit_string[0] = z;