0001385: Unsafe putenv() in mdev corrupts environment

(0002462)
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 [^]

(0002468)
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).

(0002469)
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
06-08-07 09:56	vda	Resolution	open => fixed

Notes
(0002462) 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 [^]

(0002468) 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).

(0002469) 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 ;)

Relationships