Use of uninitialized values in mail checking

[szymon . kalasz]

Configuration Information:
Machine: i686
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='i686' 
-DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='i686-pc-linux-gnu' 
-DCONF_VENDOR='pc' -DLOCALEDIR='/usr/local/share/locale' 
-DPACKAGE='bash' -DSHELL -DHAVE_CONFIG_H   -I.  -I. -I./include -I./lib  
-g -O2
uname output: Linux harmony 3.2.4-1-ARCH #1 SMP PREEMPT Sat Feb 4 
11:21:15 UTC 2012 i686 Intel(R) Core(TM)2 Duo CPU T6400 @ 2.00GHz 
GenuineIntel GNU/Linux
Machine Type: i686-pc-linux-gnu

Bash Version: 4.2
Patch Level: 29
Release Status: release


I found that behavior of bash depends on uninitialized values in 
memory.
In function file_mod_date_changed (mailcheck.c), if mailstat returns a 
non-zero
value, we are checking condition finfo.st_size == 0, where 
finfo.st_size is
uninitialized. In those rare cases when finfo.st_size is zero and
mailfiles[i]->file_size > 0 we update mailfiles array with some random 
values
(while the file may even not exist).

Consider following scenario:

I have no specified mailbox path, so it defaults to /var/mail/szymon, 
which
does not exist.
Then I run bash:

./bash
[szymon@harmony bash]$ export MAILCHECK=0
[szymon@harmony bash]$ mkdir /var/mail/szymon
You have mail in /var/mail/szymon
[szymon@harmony bash]$ rm -r /var/mail/szymon
[szymon@harmony bash]$ mkdir /var/mail/szymon
[szymon@harmony bash]$
[szymon@harmony bash]$ rm -r /var/mail/szymon
[szymon@harmony bash]$
[szymon@harmony bash]$ mkdir /var/mail/szymon
[szymon@harmony bash]$
[szymon@harmony bash]$ exit

If I modify mailstat to put some specific values into struct stat *st
(st_size = 0 and st_mtime = 1) when stat() fails, I get the following:

./bash
[szymon@harmony bash]$ export MAILCHECK=0
[szymon@harmony bash]$ mkdir /var/mail/szymon
You have mail in /var/mail/szymon
[szymon@harmony bash]$ rm -r /var/mail/szymon
[szymon@harmony bash]$ mkdir /var/mail/szymon
You have mail in /var/mail/szymon
[szymon@harmony bash]$ rm -r /var/mail/szymon
[szymon@harmony bash]$ mkdir /var/mail/szymon
You have mail in /var/mail/szymon
[szymon@harmony bash]$ rm -r /var/mail/szymon
[szymon@harmony bash]$ mkdir /var/mail/szymon
You have mail in /var/mail/szymon
[szymon@harmony bash]$ rm -r /var/mail/szymon
[szymon@harmony bash]$ exit

So behavior of bash depends on these values. valgrind confirms that 
problem:
==13079== Conditional jump or move depends on uninitialised value(s)
==13079==    at 0x809B922: check_mail (mailcheck.c:273)
==13079==    by 0x806D0E3: yyparse (parse.y:2504)
==13079==    by 0x80650AC: parse_command (eval.c:228)
==13079==    by 0x806516D: read_command (eval.c:272)
==13079==    by 0x80653A5: reader_loop (eval.c:137)
==13079==    by 0x40DE482: (below main) (in /lib/libc-2.15.so)
==13079==

It seems to be a minor bug, however, still a bug.

For my convenience, I've added some debug printfs to mailcheck.c and 
mailstat.c.
You can see the output here:
1. uninitialized: http://szymon.kalasz.student.tcs.uj.edu.pl/b1.txt
2. st_size = 0, st_mtime = 1: 
http://szymon.kalasz.student.tcs.uj.edu.pl/b2.txt

Modified mailcheck.c and mailstat.c are available here:
http://szymon.kalasz.student.tcs.uj.edu.pl/bash/



Maybe it is better to UPDATE_MAIL_FILE only when (mail)stat returns 
zero?

diff --git a/mailcheck.c b/mailcheck.c
index bd95f0d..31aa970 100644
--- a/mailcheck.c
+++ b/mailcheck.c
@@ -263,14 +263,15 @@ file_mod_date_changed (i)
   time_t mtime;
   struct stat finfo;
   char *file;
+  int r;

   file = mailfiles[i]->name;
   mtime = mailfiles[i]->mod_time;

-  if ((mailstat (file, &finfo) == 0) && (finfo.st_size > 0))
+  if (((r = mailstat (file, &finfo)) == 0) && (finfo.st_size > 0))
     return (mtime < finfo.st_mtime);

-  if (finfo.st_size == 0 && mailfiles[i]->file_size > 0)
+  if (r == 0 && finfo.st_size == 0 && mailfiles[i]->file_size > 0)
     UPDATE_MAIL_FILE (i, finfo);

   return (0);