bin/71651: [PATCH] cron may attept to close unopened file

[Giorgos Keramidas]

The following reply was made to PR bin/71651; it has been noted by GNATS.

From: Giorgos Keramidas <keramida at freebsd.org>
To: Dan Lukes <dan at obluda.cz>
Cc: bug-followup at freebsd.org
Subject: Re: bin/71651: [PATCH] cron may attept to close unopened file
Date: Sun, 12 Sep 2004 23:01:55 +0300

 On 2004-09-12 16:15, Dan Lukes <dan at obluda.cz> wrote:
 > 
 > >Number:         71651
 > >Category:       bin
 > >Synopsis:       [PATCH] cron may attept to close unopened file
 > >Confidential:   no
 > >Severity:       serious
 > >Priority:       low
 > >Responsible:    freebsd-bugs
 > >State:          open
 > >Quarter:        
 > >Keywords:       
 > >Date-Required:
 > >Class:          sw-bug
 > >Submitter-Id:   current-users
 > >Arrival-Date:   Sun Sep 12 14:20:22 GMT 2004
 > >Closed-Date:
 > >Last-Modified:
 > >Originator:     Dan Lukes
 > >Release:        FreeBSD 5.3-BETA3 i386
 > >Organization:
 > Obludarium
 > >Environment:
 > System: FreeBSD kulesh.obluda.cz 5.3-BETA3 FreeBSD 5.3-BETA3 #8: Sun Sep 5 07:06:40 CEST 2004 dan at kulesh.obluda.cz:/usr/obj/usr/src/sys/Dan i386
 > usr.sbin/cron/lib/misc.c,v 1.11 2002/08/04 04:32:27 tjr
 > usr.sbin/cron/cron/cron.c,v 1.15 2004/05/16 19:29:33 yar
 > usr.sbin/cron/cron/do_command.c,v 1.22 2004/05/16 19:29:33 yar
 > 
 > >Description:
 > usr.sbin/cron/lib/misc.c:413: warning: 'deny' might be used uninitialized in this function
 > 
 > It's sign of true bug. When fopen of ALLOW_FILE fail for other than ENOENT
 > reason, then "goto out" apply then 'if (deny)' is evaluated and
 > 'fclose(deny)' may be called athought 'deny' is uninitialized variable.
 
 The check to avoid calling fclose() with NULL is already there.  You
 just have to make sure that `allow' and `deny' are always initialized to
 NULL to let it work as expected :-)
 
 %%%
 Index: misc.c
 ===================================================================
 RCS file: /home/ncvs/src/usr.sbin/cron/lib/misc.c,v
 retrieving revision 1.11
 diff -u -r1.11 misc.c
 --- misc.c	4 Aug 2002 04:32:27 -0000	1.11
 +++ misc.c	12 Sep 2004 19:55:31 -0000
 @@ -410,7 +410,8 @@
  allowed(username)
  	char *username;
  {
 -	FILE	*allow, *deny;
 +	FILE	*allow = NULL;
 +	FILE	*deny = NULL;
  	int	isallowed;
  
  	isallowed = FALSE;
 @@ -421,9 +422,6 @@
  	if ((deny = fopen(DENY_FILE, "r")) == NULL && errno != ENOENT)
  		goto out;
  	Debug(DMISC, ("allow/deny enabled, %d/%d\n", !!allow, !!deny))
 -#else
 -	allow = NULL;
 -	deny = NULL;
  #endif
  
  	if (allow)
 %%%
 
 >   	struct tm	otztm; /* time in the old time zone */
 > - 	int		otzminute, otzhour, otzdom, otzmonth, otzdow;
 > + 	int		otzminute = otzminute, /* "init" to avoid "might be used uninitialized" warning */
 > + 		otzhour = otzhour, otzdom = otzdom, 
 > + 		otzmonth = otzmonth, otzdow = otzmonth;
 
 Please don't use this.  There's probably a true bug hidden here.
 Hiding it is not good.
 
 >   		if (ch != EOF) {
 > - 			register FILE	*mail;
 > + 			register FILE	*mail = mail; /* "init" to avoid "might be used uninitialized" warning */
 
 Use NULL as the initialization of (FILE *) objects.  If they are indeed
 used before a proper initialization is done this will expose the true
 bugs and let us fix them ;-)
 
 - Giorgos