Skip to content

Commit 7ac36ec

Browse files
committed
Cherry-pick 4cc7476
Headers: forbid \r and \n also after \0, allow CRLF followed by HT or SP and forbid \0. See bug #60227. Conflicts: ext/standard/tests/general_functions/bug60227.phpt ext/standard/tests/general_functions/bug60227_1.phpt ext/standard/tests/general_functions/bug60227_2.phpt main/SAPI.c
1 parent 8ccf065 commit 7ac36ec

File tree

5 files changed

+49
-12
lines changed

5 files changed

+49
-12
lines changed

ext/standard/tests/general_functions/bug60227_1.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ header("X-Foo5: e\rSet-Cookie: ID=123");
1010
echo 'foo';
1111
?>
1212
--EXPECTF--
13-
Warning: Header may not contain more than a single header, new line detected. in %s on line %d
13+
Warning: Header may not contain more than a single header, new line detected in %s on line %d
1414
foo
1515
--EXPECTHEADERS--
1616
X-Foo1: a

ext/standard/tests/general_functions/bug60227_2.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ header("X-Foo6: e\rSet-Cookie: ID=123\n d");
77
echo 'foo';
88
?>
99
--EXPECTF--
10-
Warning: Header may not contain more than a single header, new line detected. in %s on line %d
10+
Warning: Header may not contain more than a single header, new line detected in %s on line %d
1111
foo
1212
--EXPECTHEADERS--
1313
X-foo: e
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
--TEST--
2+
Bug #60227 (header() cannot detect the multi-line header with CR), \0 before \n
3+
--FILE--
4+
<?php
5+
header("X-foo: e\n foo");
6+
header("X-Foo6: e\0Set-Cookie: ID=\n123\n d");
7+
echo 'foo';
8+
?>
9+
--EXPECTF--
10+
Warning: Header may not contain NUL bytes in %s on line %d
11+
foo
12+
--EXPECTHEADERS--
13+
X-foo: e
14+
foo
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
--TEST--
2+
Bug #60227 (header() cannot detect the multi-line header with CR), CRLF
3+
--FILE--
4+
<?php
5+
header("X-foo: e\r\n foo");
6+
header("X-foo: e\r\nfoo");
7+
echo 'foo';
8+
?>
9+
--EXPECTF--
10+
Warning: Header may not contain more than a single header, new line detected in %s on line %d
11+
foo
12+
--EXPECTHEADERS--
13+
X-foo: e
14+
foo

main/SAPI.c

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -590,17 +590,26 @@ SAPI_API int sapi_header_op(sapi_header_op_enum op, void *arg TSRMLS_DC)
590590
return FAILURE;
591591
}
592592
} else {
593-
/* new line safety check */
594-
char *s = header_line;
595-
while (s = strpbrk(s, "\n\r")) {
596-
if (s[1] == ' ' || s[1] == '\t') {
597-
/* RFC 2616 allows new lines if followed by SP or HT */
598-
s++;
599-
continue;
593+
/* new line/NUL character safety check */
594+
int i;
595+
for (i = 0; i < header_line_len; i++) {
596+
/* RFC 2616 allows new lines if followed by SP or HT */
597+
int illegal_break =
598+
(header_line[i+1] != ' ' && header_line[i+1] != '\t')
599+
&& (
600+
header_line[i] == '\n'
601+
|| (header_line[i] == '\r' && header_line[i+1] != '\n'));
602+
if (illegal_break) {
603+
efree(header_line);
604+
sapi_module.sapi_error(E_WARNING, "Header may not contain "
605+
"more than a single header, new line detected");
606+
return FAILURE;
607+
}
608+
if (header_line[i] == '\0') {
609+
efree(header_line);
610+
sapi_module.sapi_error(E_WARNING, "Header may not contain NUL bytes");
611+
return FAILURE;
600612
}
601-
efree(header_line);
602-
sapi_module.sapi_error(E_WARNING, "Header may not contain more than a single header, new line detected.");
603-
return FAILURE;
604613
}
605614
}
606615

0 commit comments

Comments
 (0)