Fixed bug #67399 (putenv with empty variable may lead to crash)

3 files changed, 65 insertions, 56 deletions

diff --git a/NEWS b/NEWS
index 512aba8024..0c95f64ac0 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,7 @@ PHP                                                                        NEWS
     (static::) in some cases). (Levi Morrison)
   . Fixed bug #67390 (insecure temporary file use in the configure script).
     (Remi) (CVE-2014-3981)
+  . Fixed bug #67399 (putenv with empty variable may lead to crash). (Stas)
 
 - Date:
   . Fixed bug #67308 (Serialize of DateTime truncates fractions of second).
diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c
index c01365589e..4d4354d4ad 100644
--- a/ext/standard/basic_functions.c
+++ b/ext/standard/basic_functions.c
@@ -4021,92 +4021,91 @@ PHP_FUNCTION(putenv)
 {
 	char *setting;
 	int setting_len;
+	char *p, **env;
+	putenv_entry pe;
+#ifdef PHP_WIN32
+	char *value = NULL;
+	int equals = 0;
+	int error_code;
+#endif
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &setting, &setting_len) == FAILURE) {
 		return;
 	}
+    
+    if(setting_len == 0 || setting[0] == '=') {
+    	php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid parameter syntax");
+    	RETURN_FALSE;
+    }
 
-	if (setting_len) {
-		char *p, **env;
-		putenv_entry pe;
+	pe.putenv_string = estrndup(setting, setting_len);
+	pe.key = estrndup(setting, setting_len);
+	if ((p = strchr(pe.key, '='))) {	/* nullify the '=' if there is one */
+		*p = '\0';
 #ifdef PHP_WIN32
-		char *value = NULL;
-		int equals = 0;
-		int error_code;
+		equals = 1;
 #endif
+	}
 
-		pe.putenv_string = estrndup(setting, setting_len);
-		pe.key = estrndup(setting, setting_len);
-		if ((p = strchr(pe.key, '='))) {	/* nullify the '=' if there is one */
-			*p = '\0';
-#ifdef PHP_WIN32
-			equals = 1;
-#endif
-		}
-
-		pe.key_len = strlen(pe.key);
+	pe.key_len = strlen(pe.key);
 #ifdef PHP_WIN32
-		if (equals) {
-			if (pe.key_len < setting_len - 1) {
-				value = p + 1;
-			} else {
-				/* empty string*/
-				value = p;
-			}
+	if (equals) {
+		if (pe.key_len < setting_len - 1) {
+			value = p + 1;
+		} else {
+			/* empty string*/
+			value = p;
 		}
+	}
 #endif
 
-		zend_hash_del(&BG(putenv_ht), pe.key, pe.key_len+1);
+	zend_hash_del(&BG(putenv_ht), pe.key, pe.key_len+1);
 
-		/* find previous value */
-		pe.previous_value = NULL;
-		for (env = environ; env != NULL && *env != NULL; env++) {
-			if (!strncmp(*env, pe.key, pe.key_len) && (*env)[pe.key_len] == '=') {	/* found it */
+	/* find previous value */
+	pe.previous_value = NULL;
+	for (env = environ; env != NULL && *env != NULL; env++) {
+		if (!strncmp(*env, pe.key, pe.key_len) && (*env)[pe.key_len] == '=') {	/* found it */
 #if defined(PHP_WIN32)
-				/* must copy previous value because MSVCRT's putenv can free the string without notice */
-				pe.previous_value = estrdup(*env);
+			/* must copy previous value because MSVCRT's putenv can free the string without notice */
+			pe.previous_value = estrdup(*env);
 #else
-				pe.previous_value = *env;
+			pe.previous_value = *env;
 #endif
-				break;
-			}
+			break;
 		}
+	}
 
 #if HAVE_UNSETENV
-		if (!p) { /* no '=' means we want to unset it */
-			unsetenv(pe.putenv_string);
-		}
-		if (!p || putenv(pe.putenv_string) == 0) { /* success */
+	if (!p) { /* no '=' means we want to unset it */
+		unsetenv(pe.putenv_string);
+	}
+	if (!p || putenv(pe.putenv_string) == 0) { /* success */
 #else
 # ifndef PHP_WIN32
-		if (putenv(pe.putenv_string) == 0) { /* success */
+	if (putenv(pe.putenv_string) == 0) { /* success */
 # else
-		error_code = SetEnvironmentVariable(pe.key, value);
+	error_code = SetEnvironmentVariable(pe.key, value);
 #  if _MSC_VER < 1500
-		/* Yet another VC6 bug, unset may return env not found */
-		if (error_code != 0 || 
-			(error_code == 0 && GetLastError() == ERROR_ENVVAR_NOT_FOUND)) {
+	/* Yet another VC6 bug, unset may return env not found */
+	if (error_code != 0 || 
+		(error_code == 0 && GetLastError() == ERROR_ENVVAR_NOT_FOUND)) {
 #  else
-		if (error_code != 0) { /* success */
+	if (error_code != 0) { /* success */
 #  endif
 # endif
 #endif
-			zend_hash_add(&BG(putenv_ht), pe.key, pe.key_len + 1, (void **) &pe, sizeof(putenv_entry), NULL);
+		zend_hash_add(&BG(putenv_ht), pe.key, pe.key_len + 1, (void **) &pe, sizeof(putenv_entry), NULL);
 #ifdef HAVE_TZSET
-			if (!strncmp(pe.key, "TZ", pe.key_len)) {
-				tzset();
-			}
-#endif
-			RETURN_TRUE;
-		} else {
-			efree(pe.putenv_string);
-			efree(pe.key);
-			RETURN_FALSE;
+		if (!strncmp(pe.key, "TZ", pe.key_len)) {
+			tzset();
 		}
+#endif
+		RETURN_TRUE;
+	} else {
+		efree(pe.putenv_string);
+		efree(pe.key);
+		RETURN_FALSE;
 	}
-
-	php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid parameter syntax");
-	RETURN_FALSE;
 }
 /* }}} */
 #endif
diff --git a/ext/standard/tests/general_functions/putenv.phpt b/ext/standard/tests/general_functions/putenv.phpt
index afe1badce4..254207320b 100644
--- a/ext/standard/tests/general_functions/putenv.phpt
+++ b/ext/standard/tests/general_functions/putenv.phpt
@@ -15,6 +15,9 @@ var_dump(getenv($var_name));
 var_dump(putenv($var_name));
 var_dump(getenv($var_name));
 
+var_dump(putenv("=123"));
+var_dump(putenv(""));
+
 echo "Done\n";
 ?>
 --EXPECTF--	
@@ -25,4 +28,10 @@ bool(true)
 string(0) ""
 bool(true)
 bool(false)
+
+Warning: putenv(): Invalid parameter syntax in %s on line %d
+bool(false)
+
+Warning: putenv(): Invalid parameter syntax in %s on line %d
+bool(false)
 Done