From 04e8e55f47db1c209bd3c5f623a12284de44b31c Mon Sep 17 00:00:00 2001 From: Saki Takamachi Date: Mon, 4 Mar 2024 21:30:07 +0900 Subject: [PATCH] Added validation of `\n` in $additional_headers of mail() When $additional_headers of mail() is an array, the same validation as `\r\n` is now applied to `\n` alone too. --- NEWS | 2 + UPGRADING | 2 + ext/standard/mail.c | 53 ++++++++++++++++++++---- ext/standard/php_mail.h | 7 ++++ ext/standard/tests/mail/gh13415.phpt | 52 +++++++++++++++++++++++ ext/standard/tests/mail/mail_basic7.phpt | 2 +- 6 files changed, 109 insertions(+), 9 deletions(-) create mode 100644 ext/standard/tests/mail/gh13415.phpt diff --git a/NEWS b/NEWS index 3f6a05ade1c..2f32dd10f9f 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,8 @@ PHP NEWS - Standard: . Fixed bug GH-11808 (Live filesystem modified by tests). (nielsdos) + . Fixed GH-13402: Added validation of `\n` in `$additional_headers` of `mail()` + (SakiTakamachi) - XML: . Fixed bug GH-13517 (Multiple test failures when building with diff --git a/UPGRADING b/UPGRADING index b025364400f..efa74cb42cf 100644 --- a/UPGRADING +++ b/UPGRADING @@ -243,6 +243,8 @@ PHP 8.2 UPGRADE NOTES objects. . mail() function reverts back to the mixed LF and CRLF new lines (behavior before PHP 8.0) if mail.mixed_lf_and_crlf INI is on. + . When $additional_headers of mail() is an array, the same validation as + `\r\n` is now applied to `\n` alone too. - XML . xml_parser_set_option() now actually returns false when attempting to set a diff --git a/ext/standard/mail.c b/ext/standard/mail.c index 0323341c3d6..0ddec8d7231 100644 --- a/ext/standard/mail.c +++ b/ext/standard/mail.c @@ -57,7 +57,7 @@ extern zend_long php_getuid(void); -static bool php_mail_build_headers_check_field_value(zval *val) +static php_mail_header_value_error_type php_mail_build_headers_check_field_value(zval *val) { size_t len = 0; zend_string *value = Z_STR_P(val); @@ -66,20 +66,39 @@ static bool php_mail_build_headers_check_field_value(zval *val) /* https://tools.ietf.org/html/rfc2822#section-2.2.3 */ while (len < value->len) { if (*(value->val+len) == '\r') { + if (*(value->val+len+1) != '\n') { + return CONTAINS_CR_ONLY; + } + if (value->len - len >= 3 - && *(value->val+len+1) == '\n' && (*(value->val+len+2) == ' ' || *(value->val+len+2) == '\t')) { len += 3; continue; } - return FAILURE; + + return CONTAINS_CRLF; + } + /** + * The RFC does not allow using LF alone for folding. However, LF is + * often treated similarly to CRLF, and there are likely many user + * environments that use LF for folding. + * Therefore, considering such an environment, folding with LF alone + * is allowed. + */ + if (*(value->val+len) == '\n') { + if (value->len - len >= 2 + && (*(value->val+len+1) == ' ' || *(value->val+len+1) == '\t')) { + len += 2; + continue; + } + return CONTAINS_LF_ONLY; } if (*(value->val+len) == '\0') { - return FAILURE; + return CONTAINS_NULL; } len++; } - return SUCCESS; + return NO_HEADER_ERROR; } @@ -108,9 +127,27 @@ static void php_mail_build_headers_elem(smart_str *s, zend_string *key, zval *va zend_value_error("Header name \"%s\" contains invalid characters", ZSTR_VAL(key)); return; } - if (php_mail_build_headers_check_field_value(val) != SUCCESS) { - zend_value_error("Header \"%s\" has invalid format, or contains invalid characters", ZSTR_VAL(key)); - return; + + php_mail_header_value_error_type error_type = php_mail_build_headers_check_field_value(val); + switch (error_type) { + case NO_HEADER_ERROR: + break; + case CONTAINS_LF_ONLY: + zend_value_error("Header \"%s\" contains LF character that is not allowed in the header", ZSTR_VAL(key)); + return; + case CONTAINS_CR_ONLY: + zend_value_error("Header \"%s\" contains CR character that is not allowed in the header", ZSTR_VAL(key)); + return; + case CONTAINS_CRLF: + zend_value_error("Header \"%s\" contains CRLF characters that are used as a line separator and are not allowed in the header", ZSTR_VAL(key)); + return; + case CONTAINS_NULL: + zend_value_error("Header \"%s\" contains NULL character that is not allowed in the header", ZSTR_VAL(key)); + return; + default: + // fallback + zend_value_error("Header \"%s\" has invalid format, or contains invalid characters", ZSTR_VAL(key)); + return; } smart_str_append(s, key); smart_str_appendl(s, ": ", 2); diff --git a/ext/standard/php_mail.h b/ext/standard/php_mail.h index 4e2f57749dd..509233692e0 100644 --- a/ext/standard/php_mail.h +++ b/ext/standard/php_mail.h @@ -49,5 +49,12 @@ do { \ } \ } while(0) +typedef enum { + NO_HEADER_ERROR, + CONTAINS_LF_ONLY, + CONTAINS_CR_ONLY, + CONTAINS_CRLF, + CONTAINS_NULL +} php_mail_header_value_error_type; #endif /* PHP_MAIL_H */ diff --git a/ext/standard/tests/mail/gh13415.phpt b/ext/standard/tests/mail/gh13415.phpt new file mode 100644 index 00000000000..f507259a8e2 --- /dev/null +++ b/ext/standard/tests/mail/gh13415.phpt @@ -0,0 +1,52 @@ +--TEST-- +GH-13415 (Added validation of line breaks \n in $additional_headers of mail()) +--INI-- +sendmail_path={MAIL:gh13415.out} +--FILE-- + "foo@example.com \nCc: hacker@example.com"]); +} catch (Throwable $e) { + echo $e->getMessage()."\n\n"; +} + +echo "CR only:\n"; +try { + mail('to@example.com', 'Test Subject', 'A Message', ['Reply-To' => "foo@example.com \rCc: hacker@example.com"]); +} catch (Throwable $e) { + echo $e->getMessage()."\n\n"; +} + +echo "CRLF:\n"; +try { + mail('to@example.com', 'Test Subject', 'A Message', ['Reply-To' => "foo@example.com \r\nCc: hacker@example.com"]); +} catch (Throwable $e) { + echo $e->getMessage()."\n\n"; +} + +echo "NULL:\n"; +try { + mail('to@example.com', 'Test Subject', 'A Message', ['Reply-To' => "foo@example.com \0Cc: hacker@example.com"]); +} catch (Throwable $e) { + echo $e->getMessage()."\n\n"; +} +?> +--CLEAN-- + +--EXPECTF-- +LF only: +Header "Reply-To" contains LF character that is not allowed in the header + +CR only: +Header "Reply-To" contains CR character that is not allowed in the header + +CRLF: +Header "Reply-To" contains CRLF characters that are used as a line separator and are not allowed in the header + +NULL: +Header "Reply-To" contains NULL character that is not allowed in the header diff --git a/ext/standard/tests/mail/mail_basic7.phpt b/ext/standard/tests/mail/mail_basic7.phpt index 47614c011eb..d0c77f0d8f5 100644 --- a/ext/standard/tests/mail/mail_basic7.phpt +++ b/ext/standard/tests/mail/mail_basic7.phpt @@ -258,4 +258,4 @@ Subject: Test Subject foo9: %&$#! A Message -ValueError: Header "foo10" has invalid format, or contains invalid characters +ValueError: Header "foo10" contains NULL character that is not allowed in the header