[BusyBox 0001385]: Unsafe putenv() in mdev corrupts environment

[bugs at busybox.net]

The following issue has been CLOSED 
====================================================================== 
http://busybox.net/bugs/view.php?id=1385 
====================================================================== 
Reported By:                eswierk
Assigned To:                BusyBox
====================================================================== 
Project:                    BusyBox
Issue ID:                   1385
Category:                   Other
Reproducibility:            random
Severity:                   major
Priority:                   normal
Status:                     closed
Resolution:                 open
Fixed in Version:           
====================================================================== 
Date Submitted:             06-07-2007 11:34 PDT
Last Modified:              06-08-2007 09:56 PDT
====================================================================== 
Summary:                    Unsafe putenv() in mdev corrupts environment
Description: 
An unsafe use of putenv() in mdev.c (BusyBox 1.5.1) occasionally corrupts
the environment, causing the spawned process to receive garbage in the
MDEV variable.    This occurs only intermittenly, and only when mdev -s is
invoked explicitly.

If I understand the semantics of putenv(), it uses the passed string
directly, so the caller must not free it.  Patch attached.

====================================================================== 

---------------------------------------------------------------------- 
 Souf - 06-07-07 13:32  
---------------------------------------------------------------------- 
I already sent a patch for Mdev, I attach it still here, tray it.

http://busybox.net/lists/busybox/2007-June/027588.html
http://busybox.net/lists/busybox/2007-May/027515.html
http://busybox.net/lists/busybox/2007-May/027416.html
http://busybox.net/lists/busybox/2007-May/027367.html 

---------------------------------------------------------------------- 
 vda - 06-08-07 09:05  
---------------------------------------------------------------------- 
if (command) {
                int rc;
-               char *s;

-               s = xasprintf("MDEV=%s", device_name);
-               putenv(s);
+               setenv("MDEV", device_name, 1);
                rc = system(command);
-               s[4] = 0;
-               putenv(s);
-               free(s);
                free(command);
                if (rc == -1) bb_perror_msg_and_die("cannot run %s",
command);
        }

s is freed after system() call. command will never get garbled
environment.
I'd just use unsetenv() instead of putenv() to make action clear(er). 

---------------------------------------------------------------------- 
 vda - 06-08-07 09:56  
---------------------------------------------------------------------- 
Closing. Code looks like this now:

        if (command) {
                /* setenv will leak memory, so use putenv */
                char *s = xasprintf("MDEV=%s", device_name);
                putenv(s);
                if (system(command) == -1)
                        bb_perror_msg_and_die("cannot run %s", command);
                s[4] = '\0';
                unsetenv(s);
                free(s);
                free(command);
        }

Souf, your mdev patches should be revieved and applied by a maintainer who
is *actually using mdev*. Please bug them if they keep silent ;) 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
06-07-07 11:34  eswierk        New Issue                                    
06-07-07 11:34  eswierk        Status                   new => assigned     
06-07-07 11:34  eswierk        Assigned To               => BusyBox         
06-07-07 11:34  eswierk        File Added: busybox-202-mdev-putenv-bug.patch    
               
06-07-07 11:40  eswierk        Issue Monitored: eswierk                     
06-07-07 13:32  Souf           Note Added: 0002462                          
06-07-07 13:33  Souf           File Added: mdev.patch                       
06-08-07 09:05  vda            Note Added: 0002468                          
06-08-07 09:56  vda            Status                   assigned => closed  
06-08-07 09:56  vda            Note Added: 0002469                          
======================================================================