From 5497126d0522169a6b7b3ad147102344aa27a55c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sun, 27 Jan 2013 09:28:34 +0100 Subject: [PATCH] edit: Fix stack smash in sector key 'L' Key 'L' copies the source sector to a destination sector. Bug: it doesn't copy, it messes up the source sector badly instead, and can smash the stack on some machines. Root cause: doland() passes § instead of sect to ef_set_uid(). Impact: 1. ef_setuid() clobbers a few bytes at §. When the bitfield and uid fit into sizeof(sect) bytes, it clobbers just sect, which has no effect, because doland() returns without using it again. This is the case on a typical 64-bit machine: bit field and uid are both 4 bytes, sizeof(sect) is 8. When they don't fit, whatever is adjacent to sect gets clobbered. On a typical 32-bit machine with stack growing down, that's p. Again, no effect, because doland() returns without using it again. With stack growing up, it could well be the return address, crashing the server. 2. ef_setuid() fails to update *sect. Impact (when we survive 1): sect->sct_uid remains unchanged. putsect() writes to the source sector instead of the destination sector, clobbering the source's sct_x, sct_y. Breaks invariant sctoff(sct_x, sct_y) == sct_uid! Subsequent edits are all applied to the source sector. sect->sct_seqno remains unchanged. No effect, because we write to the source sector, and the unchanged sequence number is the right one there. Broken in commit 536ef0b0, v4.3.15. Signed-off-by: Markus Armbruster --- src/lib/commands/edit.c | 2 +- tests/actofgod/actofgod.xdump | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lib/commands/edit.c b/src/lib/commands/edit.c index 4842ecff..a53f4432 100644 --- a/src/lib/commands/edit.c +++ b/src/lib/commands/edit.c @@ -592,7 +592,7 @@ edit_sect(struct sctstr *sect, char *key, char *p) sect->sct_own); sect->sct_x = newx; sect->sct_y = newy; - ef_set_uid(EF_SECTOR, §, XYOFFSET(newx, newy)); + ef_set_uid(EF_SECTOR, sect, XYOFFSET(newx, newy)); break; case 'D': if (!sarg_xy(p, &newx, &newy)) diff --git a/tests/actofgod/actofgod.xdump b/tests/actofgod/actofgod.xdump index d81372cb..c6c5a841 100644 --- a/tests/actofgod/actofgod.xdump +++ b/tests/actofgod/actofgod.xdump @@ -84,7 +84,7 @@ owner xloc yloc des effic mobil off loyal terr0 terr1 terr2 terr3 dterr xdist yd 2 -6 6 4 0 0 0 0 0 0 0 0 0 -6 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 2 -4 6 4 0 0 0 0 0 0 0 0 0 -4 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 2 -2 6 4 0 0 0 0 0 0 0 0 0 -2 6 0 0 100 1 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 -0 1 -7 4 0 0 0 0 0 0 0 0 0 1 7 0 0 100 1 4 0 0 0 0 0 0 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 +0 1 7 4 0 0 0 0 0 0 0 0 0 1 7 0 0 100 1 4 0 0 0 0 0 0 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 98 3 7 4 0 0 0 0 0 0 0 0 0 3 7 0 0 100 0 4 0 0 0 0 0 98 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 98 0 0 0 0 0 2 5 7 4 0 0 0 0 0 0 0 0 0 5 7 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 0 7 7 4 0 0 0 0 0 0 0 0 0 7 7 0 0 100 0 4 0 0 0 0 0 0 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 @@ -108,7 +108,7 @@ owner xloc yloc des effic mobil off loyal terr0 terr1 terr2 terr3 dterr xdist yd 0 -6 -8 4 0 0 0 0 0 0 0 0 0 -6 -8 0 0 0 0 4 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 0 -4 -8 4 0 0 0 0 0 0 0 0 0 -4 -8 0 0 0 0 4 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 0 -2 -8 4 0 0 0 0 0 0 0 0 0 -2 -8 0 0 0 1 4 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 -3 1 -7 4 0 0 0 0 0 0 0 0 0 1 -7 0 0 100 1 4 0 0 0 0 0 3 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 +0 1 -7 4 0 0 0 0 0 0 0 0 0 1 7 0 0 100 1 4 0 0 0 0 0 0 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 3 3 -7 4 0 0 0 0 0 0 0 0 0 3 -7 0 0 100 0 4 0 0 0 0 0 3 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 3 5 -7 4 0 0 0 0 0 0 0 0 0 5 -7 0 0 100 0 4 0 0 0 0 0 3 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 3 7 -7 4 0 0 0 0 0 0 0 0 0 7 -7 0 0 100 0 4 0 0 0 0 0 3 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 @@ -180,8 +180,8 @@ owner xloc yloc des effic mobil off loyal terr0 terr1 terr2 terr3 dterr xdist yd 0 -6 -2 4 0 0 0 0 0 0 0 0 0 -6 -2 0 0 0 0 4 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 0 -4 -2 4 0 0 0 0 0 0 0 0 0 -4 -2 0 0 0 0 4 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 0 -2 -2 4 0 0 0 0 0 0 0 0 0 -2 -2 0 0 0 1 4 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 -3 1 -1 26 0 0 0 0 0 0 0 0 0 1 -1 9799 0 100 1 26 0 0 0 0 0 3 1 995 0 0 0 0 0 0 0 9999 9930 9955 0 9999 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 3 1 -1 4 0 0 0 0 0 0 0 0 0 3 -1 0 0 100 1 4 0 0 0 0 0 3 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 +3 3 -1 4 0 0 0 0 0 0 0 0 0 3 -1 0 0 100 1 4 0 0 0 0 0 3 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 3 5 -1 4 0 0 0 0 0 0 0 0 0 5 -1 0 0 100 1 4 0 0 0 0 0 3 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 3 7 -1 4 0 0 0 0 0 0 0 0 0 7 -1 0 0 100 1 4 0 0 0 0 0 3 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 3 9 -1 4 0 0 0 0 0 0 0 0 0 9 -1 0 0 100 1 4 0 0 0 0 0 3 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 @@ -489,8 +489,8 @@ uid owner type amount price maxbidder markettime xbuy ybuy xsell ysell config lost timestamp owner type id x y 0 1 0 0 11 7 -0 3 1 0 1 -1 0 3 0 0 2 -2 +0 3 1 0 1 -1 0 3 1 1 1 -1 0 3 1 2 1 -1 0 3 2 0 1 -1