bug-bash
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Use of uninitialized values in mail checking


From: szymon . kalasz
Subject: Use of uninitialized values in mail checking
Date: Thu, 21 Jun 2012 17:47:07 +0200
User-agent: Roundcube Webmail/0.5

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
address@hidden bash]$ export MAILCHECK=0
address@hidden bash]$ mkdir /var/mail/szymon
You have mail in /var/mail/szymon
address@hidden bash]$ rm -r /var/mail/szymon
address@hidden bash]$ mkdir /var/mail/szymon
address@hidden bash]$
address@hidden bash]$ rm -r /var/mail/szymon
address@hidden bash]$
address@hidden bash]$ mkdir /var/mail/szymon
address@hidden bash]$
address@hidden 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
address@hidden bash]$ export MAILCHECK=0
address@hidden bash]$ mkdir /var/mail/szymon
You have mail in /var/mail/szymon
address@hidden bash]$ rm -r /var/mail/szymon
address@hidden bash]$ mkdir /var/mail/szymon
You have mail in /var/mail/szymon
address@hidden bash]$ rm -r /var/mail/szymon
address@hidden bash]$ mkdir /var/mail/szymon
You have mail in /var/mail/szymon
address@hidden bash]$ rm -r /var/mail/szymon
address@hidden bash]$ mkdir /var/mail/szymon
You have mail in /var/mail/szymon
address@hidden bash]$ rm -r /var/mail/szymon
address@hidden 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);



reply via email to

[Prev in Thread] Current Thread [Next in Thread]