[PATCH] setproctitle: Do a deep environment copy instead of a shallow one

Jan Alexander Steffens (heftig) jan.steffens at gmail.com
Mon Jun 17 18:14:47 PDT 2013


From: "Jan Alexander Steffens (heftig)" <jan.steffens at gmail.com>

I've been seeing crashes at setproctitle.c:111 when Firefox
dlopen()s libbsd (through getaddrinfo() and libnss_wins):

108	if (eq == NULL)
109		continue;
110
111	*eq = '\0';	/* <== SIGSEGV */
112	if (setenv(envcopy[i], eq + 1, 1) < 0)
113		error = errno;
114	*eq = '=';

At first I believed that the clearenv() freeing all the environment
strings was the cause, however even with the built-in spt_clearenv(),
the crash happens. The reason why writing to the environment strings
is dangerous is still unknown to me.

Yet, doing a deep copy of the environment block instead of a shallow
copy seems to paper over the crash, so this patch does that.
---
 src/setproctitle.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/setproctitle.c b/src/setproctitle.c
index 7e4b165..0818847 100644
--- a/src/setproctitle.c
+++ b/src/setproctitle.c
@@ -86,7 +86,7 @@ spt_copyenv(int envc, char *envp[])
 	if (environ != envp)
 		return 0;
 
-	/* Make a copy of the old environ array of pointers, in case
+	/* Make a deep copy of the old environ array of pointers, in case
 	 * clearenv() or setenv() is implemented to free the internal
 	 * environ array, because we will need to access the old environ
 	 * contents to make the new copy. */
@@ -94,11 +94,13 @@ spt_copyenv(int envc, char *envp[])
 	envcopy = malloc(envsize);
 	if (envcopy == NULL)
 		return errno;
-	memcpy(envcopy, envp, envsize);
+	for (i = 0; envp[i]; i++) envcopy[i] = strdup(envp[i]);
+	envcopy[i] = NULL;
 
 	error = spt_clearenv();
 	if (error) {
 		environ = envp;
+		for(i = 0; envcopy[i]; i++) free(envcopy[i]);
 		free(envcopy);
 		return error;
 	}
@@ -116,18 +118,20 @@ spt_copyenv(int envc, char *envp[])
 		if (error) {
 #ifdef HAVE_CLEARENV
 			/* Because the old environ might not be available
-			 * anymore we will make do with the shallow copy. */
+			 * anymore we will make do with the deep copy. */
 			environ = envcopy;
 #else
 			environ = envp;
+			for(i = 0; envcopy[i]; i++) free(envcopy[i]);
 			free(envcopy);
 #endif
 			return error;
 		}
 	}
 
-	/* Dispose of the shallow copy, now that we've finished transfering
+	/* Dispose of the deep copy, now that we've finished transfering
 	 * the old environment. */
+	for(i = 0; envcopy[i]; i++) free(envcopy[i]);
 	free(envcopy);
 
 	return 0;
-- 
1.8.3.1



More information about the libbsd mailing list