empire/empserver
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

give: Fix integer overflows on silly third arguments

Browse source 
give() silently caps the resulting number of items to 0..ITEM_MAX.

However, its test for "< 0" suffers integer overflow on two's
complement machines (i.e. practically everywhere) when the amount
argument is INT_MIN.  give() proceeds as if the result was in range:
it sets the number of items to (short)(n + INT_MIN), telexes the owner
that INT_MIN items were stolen (obviously bogus), and tells the deity
that there are now n + INT_MIN items in X,Y.

On common machines, (short)(n + INT_MIN) == n, i.e. nothing is given.

On an oddball machine with short as wide as int, the cast to short
does nothing, item_prewrite() oopses, and corrects the number of items
to zero.

In both cases, output and telegram lie.

Likewise, its test for "> ITEM_MAX" suffers integer overflow for
sufficiently big amount arguments.  Again, give() proceeds as if the
result was in range: it sets the number of items to (short)(n + amt),
telexes the owner that -amt items were stolen (obviously bogus), and
tells the deity that there are now close to INT_MIN items in X,Y.

On common machines, (short)(n + amt) = n + INT_MAX - amt - 1,
i.e. some items are stolen.

On an oddball machine with short as wide as int, the cast to short
does nothing, item_prewrite() oopses, and corrects the number of items
to zero.

Again, output and telegram lie.

Aside: setsector can suffer similar overflows, but it reports the
resulting change correctly.  Good enough.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
This commit is contained in:
Markus Armbruster 2013-01-20 16:39:32 +01:00
parent 546e1220e0
commit 9aaf609359
3 changed files with 10 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

7
src/lib/commands/give.c
View file

@ -29,6 +29,7 @@
* Known contributors to this file:
* David Muir Sharnoff
* Steve McClure, 1997
* Markus Armbruster, 2004-2013
*/
#include <config.h>
@ -65,11 +66,11 @@ give(void)
if (!check_sect_ok(&sect))
return RET_FAIL;
n = sect.sct_item[ip->i_uid];
if (amt < 0 && -amt > n) {
if (amt < 0 && n + amt < 0)
m = 0;
} else if (amt > 0 && amt + n > ITEM_MAX) {
else if (amt > 0 && n > ITEM_MAX - amt)
m = ITEM_MAX;
} else
else
m = n + amt;
sect.sct_item[ip->i_uid] = m;
putsect(&sect);

6
tests/actofgod/actofgod.xdump
View file

@ -75,7 +75,7 @@ owner xloc yloc des effic mobil off loyal terr0 terr1 terr2 terr3 dterr xdist yd
0 0 6 0 0 0 0 0 0 0 0 0 0 0 6 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 0 0 0 0 0
1 2 6 4 0 0 0 0 0 0 0 0 0 2 6 0 0 100 1 4 0 0 0 0 0 1 1 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 0 0 0 0 0
1 4 6 4 0 0 0 0 0 0 0 0 0 4 6 0 0 100 0 4 0 0 0 0 0 1 2 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 0 0 0 0 0
1 6 6 4 0 0 0 0 0 0 0 0 0 6 6 0 0 100 0 4 0 0 0 0 0 1 1 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 0 0 0 0 0
1 6 6 4 0 0 0 0 0 0 0 0 0 6 6 0 0 100 0 4 0 0 0 0 0 1 9999 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 0 0 0 0 0
1 8 6 4 0 0 0 0 0 0 0 0 0 8 6 0 0 100 0 4 0 0 0 0 0 1 1 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 0 0 0 0 0
1 10 6 4 0 0 0 0 0 0 0 0 0 10 6 0 0 100 0 4 0 0 0 0 0 1 1 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 0 0 0 0 0
2 -12 6 4 0 0 0 0 0 0 0 0 0 -12 6 0 0 100 0 4 0 0 0 0 0 2 1 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 healthy 0 0 0 0 0 0 0 0
@ -364,8 +364,8 @@ actor action victim times duration time
0 43 98 1 0 0
0 43 2 1 0 0
0 43 1 54 0 0
1 45 0 3 0 0
0 42 1 3 0 0
0 42 1 4 0 0
1 45 0 1 0 0
0 44 2 1 0 0
0 43 3 1 0 0
/config

7
tests/actofgod/journal.log
View file

@ -752,7 +752,7 @@
Play#0 output Play#0 6 0 640
Play#0 input give l 2,6 -2147483648
Play#0 command give
Play#0 output Play#0 1 -2147483648 light products in 2,6
Play#0 output Play#0 1 0 light products in 2,6
Play#0 output Play#0 6 0 640
Play#0 input give c 4:8,6 1
Play#0 command give
@ -762,7 +762,7 @@
Play#0 output Play#0 6 0 640
Play#0 input give c 6,6 2147483647
Play#0 command give
Play#0 output Play#0 1 -2147483647 civilians in 6,6
Play#0 output Play#0 1 9999 civilians in 6,6
Play#0 output Play#0 6 0 640
Play#0 input give c 8,6 -1
Play#0 command give
@ -1535,11 +1535,10 @@
Play#0 output Play#0 1 Available workforce in 1,5 was changed from 0 to 1 by an act of POGO
Play#0 output Play#0 1 Mobility in 3,5 was changed from 0 to 2 by an act of POGO
Play#0 output Play#0 1 Available workforce in 3,5 was changed from 0 to 1 by an act of POGO
Play#0 output Play#0 1 POGO stole -2147483648 light products from 2,6
Play#0 output Play#0 1 POGO gave you 1 civilians in 4,6
Play#0 output Play#0 1 POGO gave you 1 civilians in 6,6
Play#0 output Play#0 1 POGO gave you 1 civilians in 8,6
Play#0 output Play#0 1 POGO stole -2147483647 civilians from 6,6
Play#0 output Play#0 1 POGO gave you 9997 civilians in 6,6
Play#0 output Play#0 1 POGO stole 1 civilians from 8,6
Play#0 output Play#0 1 Military reserves changed from 0 to 0 by divine intervention.
Play#0 output Play#0 1 Money changed from 0 to -2147483648 by divine intervention.