Fixed bug #77612

Port php_setcookie() to use the smart_str API to ensure that there
can be no string truncation issues.

Author: nikic

NEWS

@@ -36,6 +36,8 @@ PHP                                                                        NEWS
 - Standard:
   . Fixed bug #77552 (Unintialized php_stream_statbuf in stat functions).
     (John Stevenson)
+  . Fixed bug #77612 (setcookie() sets incorrect SameSite header if all of its
+    options filled). (Nikita)
 
 - MySQL
   . Disabled LOCAL INFILE by default, can be enabled using php.ini directive

ext/standard/head.c

@@ -30,6 +30,7 @@
 #endif
 
 #include "php_globals.h"
+#include "zend_smart_str.h"
 
 
 /* Implementation of the language Header() function */
@@ -81,12 +82,10 @@ PHPAPI int php_header(void)
 
 PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain, int secure, int httponly, zend_string *samesite, int url_encode)
 {
-	char *cookie;
-	size_t len = sizeof("Set-Cookie: ");
 	zend_string *dt;
 	sapi_header_line ctr = {0};
 	int result;
-	zend_string *encoded_value = NULL;
+	smart_str buf = {0};
 
 	if (!ZSTR_LEN(name)) {
 		zend_error( E_WARNING, "Cookie names must not be empty" );
@@ -112,97 +111,82 @@ PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires,
 		return FAILURE;
 	}
 
-	len += ZSTR_LEN(name);
-	if (value) {
-		if (url_encode) {
-			encoded_value = php_url_encode(ZSTR_VAL(value), ZSTR_LEN(value));
-			len += ZSTR_LEN(encoded_value);
-		} else {
-			encoded_value = zend_string_copy(value);
-			len += ZSTR_LEN(encoded_value);
-		}
-	}
-
-	if (path) {
-		len += ZSTR_LEN(path);
-	}
-	if (domain) {
-		len += ZSTR_LEN(domain);
-	}
-	if (samesite) {
-		len += ZSTR_LEN(samesite);
-	}
-
-	cookie = emalloc(len + 100);
-
 	if (value == NULL || ZSTR_LEN(value) == 0) {
 		/*
 		 * MSIE doesn't delete a cookie when you set it to a null value
 		 * so in order to force cookies to be deleted, even on MSIE, we
 		 * pick an expiry date in the past
 		 */
 		dt = php_format_date("D, d-M-Y H:i:s T", sizeof("D, d-M-Y H:i:s T")-1, 1, 0);
-		snprintf(cookie, len + 100, "Set-Cookie: %s=deleted; expires=%s; Max-Age=0", ZSTR_VAL(name), ZSTR_VAL(dt));
+		smart_str_appends(&buf, "Set-Cookie: ");
+		smart_str_append(&buf, name);
+		smart_str_appends(&buf, "=deleted; expires=");
+		smart_str_append(&buf, dt);
+		smart_str_appends(&buf, "; Max-Age=0");
 		zend_string_free(dt);
 	} else {
-		snprintf(cookie, len + 100, "Set-Cookie: %s=%s", ZSTR_VAL(name), value ? ZSTR_VAL(encoded_value) : "");
+		smart_str_appends(&buf, "Set-Cookie: ");
+		smart_str_append(&buf, name);
+		smart_str_appendc(&buf, '=');
+		if (url_encode) {
+			zend_string *encoded_value = php_url_encode(ZSTR_VAL(value), ZSTR_LEN(value));
+			smart_str_append(&buf, encoded_value);
+			zend_string_release_ex(encoded_value, 0);
+		} else {
+			smart_str_append(&buf, value);
+		}
 		if (expires > 0) {
 			const char *p;
-			char tsdelta[13];
 			double diff;
 
-			strlcat(cookie, COOKIE_EXPIRES, len + 100);
+			smart_str_appends(&buf, COOKIE_EXPIRES);
 			dt = php_format_date("D, d-M-Y H:i:s T", sizeof("D, d-M-Y H:i:s T")-1, expires, 0);
 			/* check to make sure that the year does not exceed 4 digits in length */
 			p = zend_memrchr(ZSTR_VAL(dt), '-', ZSTR_LEN(dt));
 			if (!p || *(p + 5) != ' ') {
 				zend_string_free(dt);
-				efree(cookie);
-				zend_string_release_ex(encoded_value, 0);
+				smart_str_free(&buf);
 				zend_error(E_WARNING, "Expiry date cannot have a year greater than 9999");
 				return FAILURE;
 			}
-			strlcat(cookie, ZSTR_VAL(dt), len + 100);
+
+			smart_str_append(&buf, dt);
 			zend_string_free(dt);
 
 			diff = difftime(expires, time(NULL));
 			if (diff < 0) {
 				diff = 0;
 			}
-			snprintf(tsdelta, sizeof(tsdelta), ZEND_LONG_FMT, (zend_long) diff);
-			strlcat(cookie, COOKIE_MAX_AGE, len + 100);
-			strlcat(cookie, tsdelta, len + 100);
-		}
-	}
 
-	if (encoded_value) {
-		zend_string_release_ex(encoded_value, 0);
+			smart_str_appends(&buf, COOKIE_MAX_AGE);
+			smart_str_append_long(&buf, (zend_long) diff);
+		}
 	}
 
 	if (path && ZSTR_LEN(path)) {
-		strlcat(cookie, COOKIE_PATH, len + 100);
-		strlcat(cookie, ZSTR_VAL(path), len + 100);
+		smart_str_appends(&buf, COOKIE_PATH);
+		smart_str_append(&buf, path);
 	}
 	if (domain && ZSTR_LEN(domain)) {
-		strlcat(cookie, COOKIE_DOMAIN, len + 100);
-		strlcat(cookie, ZSTR_VAL(domain), len + 100);
+		smart_str_appends(&buf, COOKIE_DOMAIN);
+		smart_str_append(&buf, domain);
 	}
 	if (secure) {
-		strlcat(cookie, COOKIE_SECURE, len + 100);
+		smart_str_appends(&buf, COOKIE_SECURE);
 	}
 	if (httponly) {
-		strlcat(cookie, COOKIE_HTTPONLY, len + 100);
+		smart_str_appends(&buf, COOKIE_HTTPONLY);
 	}
 	if (samesite && ZSTR_LEN(samesite)) {
-		strlcat(cookie, COOKIE_SAMESITE, len + 100);
-		strlcat(cookie, ZSTR_VAL(samesite), len + 100);
+		smart_str_appends(&buf, COOKIE_SAMESITE);
+		smart_str_append(&buf, samesite);
 	}
 
-	ctr.line = cookie;
-	ctr.line_len = (uint32_t)strlen(cookie);
+	ctr.line = ZSTR_VAL(buf.s);
+	ctr.line_len = (uint32_t) ZSTR_LEN(buf.s);
 
 	result = sapi_header_op(SAPI_HEADER_ADD, &ctr);
-	efree(cookie);
+	zend_string_release(buf.s);
 	return result;
 }