Date: Thu, 21 Aug 2003 15:17:31 -0600 From: Greg Lewis <glewis@misty.eyesbeyond.com> To: freebsd-audit@freebsd.org Subject: Fix buffer overflow in rogue (misc/43886) Message-ID: <20030821211731.GA61715@misty.eyesbeyond.com>
next in thread | raw e-mail | index | archive | help
--nFreZHaLTZJo0R7j Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi all, Restoring save files for rogue(6) has a potential buffer overflow allowing the attacker to gain gid games. See PR misc/43886. As penance for enabling rogue in the freebsd-games port I've ported over the NetBSD fixes for this buffer overflow and committed them to the port. However, rogue is still unpatched in -STABLE. The attached patch addresses the problem in -STABLE. In addition, it includes the spelling fix from bin/45701. Note that I've only ported over the buffer overflow fix, not the rest of the code cleanup which was done in NetBSD. I'd appreciate it if someone could review the fix and preferably commit it before the imminent 4.9 code freeze (as it is a security fix, albeit a relatively minor one in the scheme of things). -- Greg Lewis Email : glewis@eyesbeyond.com Eyes Beyond Web : http://www.eyesbeyond.com Information Technology FreeBSD : glewis@FreeBSD.org --nFreZHaLTZJo0R7j Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="rogue.diff" Index: rogue/inventory.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/inventory.c,v retrieving revision 1.4 diff -u -r1.4 inventory.c --- rogue/inventory.c 30 Nov 1999 03:49:23 -0000 1.4 +++ rogue/inventory.c 21 Aug 2003 19:58:45 -0000 @@ -415,14 +415,14 @@ mix_colors() { short i, j, k; - char *t; + char t[MAX_ID_TITLE_LEN]; for (i = 0; i <= 32; i++) { j = get_rand(0, (POTIONS - 1)); k = get_rand(0, (POTIONS - 1)); - t = id_potions[j].title; - id_potions[j].title = id_potions[k].title; - id_potions[k].title = t; + memcpy(t, id_potions[j].title, MAX_ID_TITLE_LEN); + memcpy(id_potions[j].title, id_potions[k].title, MAX_ID_TITLE_LEN); + memcpy(id_potions[k].title, t, MAX_ID_TITLE_LEN); } } Index: rogue/message.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/message.c,v retrieving revision 1.7.2.1 diff -u -r1.7.2.1 message.c --- rogue/message.c 20 Jul 2000 10:35:07 -0000 1.7.2.1 +++ rogue/message.c 21 Aug 2003 19:58:51 -0000 @@ -60,7 +60,7 @@ char msgs[NMESSAGES][DCOLS] = {"", "", "", "", ""}; short msg_col = 0, imsg = -1; boolean msg_cleared = 1, rmsg = 0; -char hunger_str[8] = ""; +char hunger_str[HUNGER_STR_LEN] = ""; const char *more = "-more-"; extern boolean cant_int, did_int, interrupted, save_is_interactive, flush; Index: rogue/move.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/move.c,v retrieving revision 1.7 diff -u -r1.7 move.c --- rogue/move.c 30 Nov 1999 03:49:24 -0000 1.7 +++ rogue/move.c 21 Aug 2003 20:02:38 -0000 @@ -64,7 +64,6 @@ extern short cur_level, max_level; extern short bear_trap, haste_self, confused; extern short e_rings, regeneration, auto_search; -extern char hunger_str[]; extern boolean being_held, interrupted, r_teleport, passgo; one_move_rogue(dirch, pickup) Index: rogue/object.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/object.c,v retrieving revision 1.5 diff -u -r1.5 object.c --- rogue/object.c 30 Nov 1999 03:49:25 -0000 1.5 +++ rogue/object.c 21 Aug 2003 20:04:13 -0000 @@ -159,7 +159,6 @@ extern short cur_level, max_level; extern short party_room; -extern char *error_file; extern boolean is_wood[]; put_objects() Index: rogue/pack.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/pack.c,v retrieving revision 1.8 diff -u -r1.8 pack.c --- rogue/pack.c 30 Nov 1999 03:49:25 -0000 1.8 +++ rogue/pack.c 21 Aug 2003 20:07:28 -0000 @@ -342,7 +342,7 @@ char desc[DCOLS]; if (rogue.armor) { - message("your already wearing some", 0); + message("you're already wearing some", 0); return; } ch = pack_letter("wear what?", ARMOR); Index: rogue/rogue.h =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/rogue.h,v retrieving revision 1.3.2.1 diff -u -r1.3.2.1 rogue.h --- rogue/rogue.h 17 Dec 2001 12:43:23 -0000 1.3.2.1 +++ rogue/rogue.h 21 Aug 2003 21:11:28 -0000 @@ -194,9 +194,12 @@ #define MAX_OPT_LEN 40 +#define HUNGER_STR_LEN 8 + +#define MAX_ID_TITLE_LEN 64 struct id { short value; - char *title; + char title[MAX_ID_TITLE_LEN]; char *real; unsigned short id_status; }; @@ -472,3 +475,9 @@ short second; /* 0 - 59 */ }; +/* + * external variable declarations. + */ +extern char hunger_str[HUNGER_STR_LEN]; +extern char login_name[MAX_OPT_LEN]; +extern const char *error_file; Index: rogue/save.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/save.c,v retrieving revision 1.6 diff -u -r1.6 save.c --- rogue/save.c 30 Nov 1999 03:49:27 -0000 1.6 +++ rogue/save.c 21 Aug 2003 20:03:37 -0000 @@ -63,8 +63,6 @@ extern boolean detect_monster; extern short cur_level, max_level; -extern char hunger_str[]; -extern char login_name[]; extern short party_room; extern short foods; extern boolean is_wood[]; @@ -102,15 +100,23 @@ { FILE *fp; int file_id; - char name_buffer[80]; + char *name_buffer; + size_t len; char *hptr; struct rogue_time rt_buf; if (sfile[0] == '~') { if (hptr = md_getenv("HOME")) { - (void) strcpy(name_buffer, hptr); - (void) strcat(name_buffer, sfile+1); - sfile = name_buffer; + len = strlen(hptr) + strlen(sfile); + name_buffer = md_malloc(len); + if (name_buffer == NULL) { + message("out of memory for save file name", 0); + sfile = error_file; + } else { + (void) strcpy(name_buffer, hptr); + (void) strcat(name_buffer, sfile+1); + sfile = name_buffer; + } } } /* revoke */ @@ -199,10 +205,10 @@ r_read(fp, (char *) &detect_monster, sizeof(detect_monster)); r_read(fp, (char *) &cur_level, sizeof(cur_level)); r_read(fp, (char *) &max_level, sizeof(max_level)); - read_string(hunger_str, fp); + read_string(hunger_str, fp, sizeof hunger_str); - (void) strcpy(tbuf, login_name); - read_string(login_name, fp); + (void) strlcpy(tbuf, login_name, sizeof tbuf); + read_string(login_name, fp, sizeof login_name); if (strcmp(tbuf, login_name)) { clean_up("you're not the original player"); } @@ -345,7 +351,7 @@ r_read(fp, (char *) &(id_table[i].value), sizeof(short)); r_read(fp, (char *) &(id_table[i].id_status), sizeof(unsigned short)); - read_string(id_table[i].title, fp); + read_string(id_table[i].title, fp, MAX_ID_TITLE_LEN); } } } @@ -362,13 +368,16 @@ r_write(fp, s, n); } -read_string(s, fp) +read_string(s, fp, len) char *s; FILE *fp; +size_t len; { short n; r_read(fp, (char *) &n, sizeof(short)); + if (n > len) + clean_up("read_string: corrupt game file"); r_read(fp, s, n); xxxx(s, n); } Index: rogue/score.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/score.c,v retrieving revision 1.4 diff -u -r1.4 score.c --- rogue/score.c 30 Nov 1999 03:49:27 -0000 1.4 +++ rogue/score.c 21 Aug 2003 20:03:30 -0000 @@ -58,7 +58,6 @@ #include "rogue.h" #include "pathnames.h" -extern char login_name[]; extern char *m_names[]; extern short max_level; extern boolean score_only, no_skull, msg_cleared; Index: rogue/use.c =================================================================== RCS file: /var/fcvs/src/games/rogue/Attic/use.c,v retrieving revision 1.4 diff -u -r1.4 use.c --- rogue/use.c 30 Nov 1999 03:49:29 -0000 1.4 +++ rogue/use.c 21 Aug 2003 20:06:00 -0000 @@ -68,7 +68,6 @@ const char *strange_feeling = "you have a strange feeling for a moment, then it passes"; extern short bear_trap; -extern char hunger_str[]; extern short cur_room; extern long level_points[]; extern boolean being_held; --nFreZHaLTZJo0R7j--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030821211731.GA61715>