Skip to content

Commit ab7b3a2

Browse files
author
Sjoerd Langkemper
committed
Validate chunk extension in dechunk filter
Chunked transfer encoding has a chunk-size field that can be optionally followed by a chunk-ext field. This was already skipped by php_dechunk, by skipping anything that was not a hex character. This changes php_dechunk to be a little bit more strict in what it considers a chunk-ext, and treat it as an error and stop decoding if the extension does not start with optional whitespace and a semicolon. The dechunk filter is used in some filter chain attacks as an oracle that determines whether a string starts with a hex character. Anything after the hex character would be interpreted as the chunk-ext. This change make that a bit more narrow, also requiring a semicolon, thus reducing the usefulness of the dechunk filter for attackers. This only changes behavior on invalid chunked encoding; the happy flow for valid chunked encoding remains unchanged. The state CHUNK_SIZE_EXT is renamed to CHUNK_MAYBE_EXT, where the size of the chunk is read and maybe followed by a chunk-ext. In CHUNK_VALID_EXT we have detected the semicolon and just skip the rest of the line. #21983 https://httpwg.org/specs/rfc9112.html#chunked.encoding
1 parent 7c67283 commit ab7b3a2

2 files changed

Lines changed: 62 additions & 3 deletions

File tree

ext/standard/filters.c

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1781,7 +1781,8 @@ static const php_stream_filter_factory consumed_filter_factory = {
17811781
typedef enum _php_chunked_filter_state {
17821782
CHUNK_SIZE_START,
17831783
CHUNK_SIZE,
1784-
CHUNK_SIZE_EXT,
1784+
CHUNK_MAYBE_EXT,
1785+
CHUNK_VALID_EXT,
17851786
CHUNK_SIZE_CR,
17861787
CHUNK_SIZE_LF,
17871788
CHUNK_BODY,
@@ -1820,7 +1821,7 @@ static size_t php_dechunk(char *buf, size_t len, php_chunked_filter_data *data)
18201821
data->state = CHUNK_ERROR;
18211822
break;
18221823
} else {
1823-
data->state = CHUNK_SIZE_EXT;
1824+
data->state = CHUNK_MAYBE_EXT;
18241825
break;
18251826
}
18261827
data->state = CHUNK_SIZE;
@@ -1831,7 +1832,34 @@ static size_t php_dechunk(char *buf, size_t len, php_chunked_filter_data *data)
18311832
} else if (p == end) {
18321833
return out_len;
18331834
}
1834-
case CHUNK_SIZE_EXT:
1835+
// intentional fallthrough: state is CHUNK_MAYBE_EXT if we get here
1836+
ZEND_FALLTHROUGH;
1837+
case CHUNK_MAYBE_EXT:
1838+
// end of size, but chunk-ext may follow
1839+
if (*p == '\r' || *p == '\n') {
1840+
data->state = CHUNK_SIZE_CR;
1841+
continue;
1842+
} else {
1843+
// we are not at the end of the line, so we expect a valid chunk-ext
1844+
// skip whitespace
1845+
while (p < end && (*p == ' ' || *p == '\t')) {
1846+
p++;
1847+
}
1848+
if (p == end) {
1849+
return out_len;
1850+
}
1851+
1852+
// semicolon indicates start of chunk-ext
1853+
if (*p == ';') {
1854+
data->state = CHUNK_VALID_EXT;
1855+
} else {
1856+
data->state = CHUNK_ERROR;
1857+
continue;
1858+
}
1859+
}
1860+
// intentional fallthrough: state is CHUNK_VALID_EXT if we get here
1861+
ZEND_FALLTHROUGH;
1862+
case CHUNK_VALID_EXT:
18351863
/* skip extension */
18361864
while (p < end && *p != '\r' && *p != '\n') {
18371865
p++;
@@ -1915,6 +1943,12 @@ static size_t php_dechunk(char *buf, size_t len, php_chunked_filter_data *data)
19151943
p = end;
19161944
continue;
19171945
case CHUNK_ERROR:
1946+
// Unable to parse, which means that this did not look like chunked encoding.
1947+
// If we haven't output anything yet, return the buffer unchanged
1948+
if (out_len == 0) {
1949+
return len;
1950+
}
1951+
// Otherwise, append the remaining buffer and return
19181952
if (p != out) {
19191953
memmove(out, p, end - p);
19201954
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
Chunked encoding (error handling)
3+
--SKIPIF--
4+
<?php
5+
$filters = stream_get_filters();
6+
if(! in_array( "dechunk", $filters )) die( "skip Chunked filter not available." );
7+
?>
8+
--INI--
9+
allow_url_fopen=1
10+
--FILE--
11+
<?php
12+
$streams = array(
13+
"data://text/plain,apple",
14+
"data://text/plain,mango",
15+
);
16+
foreach ($streams as $name) {
17+
$fp = fopen($name, "r");
18+
stream_filter_append($fp, "dechunk", STREAM_FILTER_READ);
19+
var_dump(stream_get_contents($fp));
20+
fclose($fp);
21+
}
22+
?>
23+
--EXPECT--
24+
string(5) "apple"
25+
string(5) "mango"

0 commit comments

Comments
 (0)