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

fix #H6867 - mail buffer overrun

Browse Source 
Web contact report of a github pull request.  A previous fix from
same user dealt with potential crash caused by freeing mailbox data
when the mailbox came from getenv("MAIL").  getenv() doesn't return
a value obtained by malloc so freeing it was bad.  The fix was to
allocate memory to hold a copy of getenv("MAIL") so that free() was
valid.  Unfortunately it didn't allocate enough space to hold the
terminating '\0' so potentially corrupted malloc/free bookkeeping
data.  And the alloc+copy was being performed every time the mailbox
was checked, resulting in leaked memory from the previous check (if
MAIL came from player's environment).  Fortunately the recheck only
takes place after new mail is actually detected and reported to the
player so the leak was probably small for most folks.

This compiles for the set of conditionals that apply to me (after
taking out -DNOMAIL that the hints put in my Makefile) but I can't
test that it actually works since mail is never delivered to this
machine.
This commit is contained in:
PatR
2018-02-19 11:59:14 -08:00
parent 7a0ad0ff21
commit 859ef823d6
2 changed files with 30 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
doc/fixes36.1
View File

@@ -577,6 +577,11 @@ the fix for secret doors on special levels always having vertical orientation
the fix intended for "a shop object stolen from outside the shop (via
grappling hook) would be left marked as 'unpaid'" broke normal pickup,
preventing any picked up item from merging with compatible stack
unix: freeing mailbox data at game end crashed if MAIL came from environment
unix: fix for freeing MAIL introduced a one-byte buffer overrun which could
interfere with malloc/free operation
unix: fix for freeing MAIL also introduced a memory leak whenever new mail
is detected and MAIL comes from the environment
Platform- and/or Interface-Specific Fixes

44
src/mail.c
View File

@@ -1,4 +1,4 @@
/* NetHack 3.6 mail.c $NHDT-Date: 1464222344 2016/05/26 00:25:44 $ $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.27 $ */
/* NetHack 3.6 mail.c $NHDT-Date: 1519070343 2018/02/19 19:59:03 $ $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.31 $ */
/* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/* NetHack may be freely redistributed. See license for details. */
@@ -90,38 +90,44 @@ free_maildata()
void
getmailstatus()
{
char *emailbox;
if ((emailbox = nh_getenv("MAIL")) != 0) {
mailbox = (char *) alloc((unsigned) strlen(emailbox));
Strcpy(mailbox, emailbox);
}
if (!mailbox) {
if (mailbox) {
; /* no need to repeat the setup */
} else if ((mailbox = nh_getenv("MAIL")) != 0) {
mailbox = dupstr(mailbox);
#ifdef MAILPATH
} else {
#ifdef AMS
struct passwd ppasswd;
(void) memcpy(&ppasswd, getpwuid(getuid()), sizeof(struct passwd));
(void) memcpy(&ppasswd, getpwuid(getuid()), sizeof (struct passwd));
if (ppasswd.pw_dir) {
mailbox = (char *) alloc((unsigned) strlen(ppasswd.pw_dir)
+ sizeof(AMS_MAILBOX));
/* note: 'sizeof "LITERAL"' includes +1 for terminating '\0' */
mailbox = (char *) alloc((unsigned) (strlen(ppasswd.pw_dir)
+ sizeof AMS_MAILBOX));
Strcpy(mailbox, ppasswd.pw_dir);
Strcat(mailbox, AMS_MAILBOX);
} else
return;
}
#else
const char *pw_name = getpwuid(getuid())->pw_name;
mailbox = (char *) alloc(sizeof(MAILPATH) + strlen(pw_name));
/* note: 'sizeof "LITERAL"' includes +1 for terminating '\0' */
mailbox = (char *) alloc((unsigned) (strlen(pw_name)
+ sizeof MAILPATH));
Strcpy(mailbox, MAILPATH);
Strcat(mailbox, pw_name);
#endif /* AMS */
#else
return;
#endif
#endif /* MAILPATH */
}
if (stat(mailbox, &omstat)) {
debugpline3("mailbox=%c%s%c",
mailbox ? '\"' : '<',
mailbox ? mailbox : "null",
mailbox ? '\"' : '>');
if (mailbox && stat(mailbox, &omstat)) {
#ifdef PERMANENT_MAILBOX
pline("Cannot get status of MAIL=\"%s\".", mailbox);
mailbox = 0;
free_maildata(); /* set 'mailbox' to Null */
#else
omstat.st_mtime = 0;
#endif
@@ -495,7 +501,7 @@ ckmailstatus()
if (stat(mailbox, &nmstat)) {
#ifdef PERMANENT_MAILBOX
pline("Cannot get status of MAIL=\"%s\" anymore.", mailbox);
mailbox = 0;
free_maildata();
#else
nmstat.st_mtime = 0;
#endif