From a65e2541d25324890da161b4057b0e9b87947890 Mon Sep 17 00:00:00 2001 From: Henk te Sligte Date: Tue, 2 Oct 2018 20:31:37 +0200 Subject: [PATCH 1/3] Fixed bug #74371 strip_tags altering attributes --- ext/standard/string.c | 2 ++ ext/standard/tests/strings/bug74371.phpt | 10 ++++++++++ 2 files changed, 12 insertions(+) create mode 100644 ext/standard/tests/strings/bug74371.phpt diff --git a/ext/standard/string.c b/ext/standard/string.c index 2f20d62a7485b..feea471efd700 100644 --- a/ext/standard/string.c +++ b/ext/standard/string.c @@ -5123,6 +5123,7 @@ PHPAPI size_t php_strip_tags_ex(char *rbuf, size_t len, uint8_t *stateptr, const break; case '<': if (in_q) { + goto reg_char_1; break; } if (isspace(*(p + 1)) && !allow_tag_spaces) { @@ -5136,6 +5137,7 @@ PHPAPI size_t php_strip_tags_ex(char *rbuf, size_t len, uint8_t *stateptr, const break; } if (in_q) { + goto reg_char_1; break; } diff --git a/ext/standard/tests/strings/bug74371.phpt b/ext/standard/tests/strings/bug74371.phpt new file mode 100644 index 0000000000000..123d13e94e3be --- /dev/null +++ b/ext/standard/tests/strings/bug74371.phpt @@ -0,0 +1,10 @@ +--TEST-- +Bug #74371: strip_tags altering attributes +--FILE-- + :<">', ''); + +?> +--EXPECT-- +:> :< From 52260529a0d0822dd55015a079b3dcb258c10070 Mon Sep 17 00:00:00 2001 From: Henk te Sligte Date: Wed, 3 Oct 2018 12:56:36 +0200 Subject: [PATCH 2/3] removed unnecessary break, added some more tests --- ext/standard/string.c | 2 -- ext/standard/tests/strings/bug74371.phpt | 8 +++++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/ext/standard/string.c b/ext/standard/string.c index feea471efd700..974ea35498472 100644 --- a/ext/standard/string.c +++ b/ext/standard/string.c @@ -5124,7 +5124,6 @@ PHPAPI size_t php_strip_tags_ex(char *rbuf, size_t len, uint8_t *stateptr, const case '<': if (in_q) { goto reg_char_1; - break; } if (isspace(*(p + 1)) && !allow_tag_spaces) { goto reg_char_1; @@ -5138,7 +5137,6 @@ PHPAPI size_t php_strip_tags_ex(char *rbuf, size_t len, uint8_t *stateptr, const } if (in_q) { goto reg_char_1; - break; } lc = '>'; diff --git a/ext/standard/tests/strings/bug74371.phpt b/ext/standard/tests/strings/bug74371.phpt index 123d13e94e3be..e97711a34ee27 100644 --- a/ext/standard/tests/strings/bug74371.phpt +++ b/ext/standard/tests/strings/bug74371.phpt @@ -3,8 +3,14 @@ Bug #74371: strip_tags altering attributes --FILE-- :<">', ''); +echo strip_tags(':> :<', '') . PHP_EOL; +echo strip_tags('\':> :<', '') . PHP_EOL; +echo strip_tags(':', '') . PHP_EOL; +echo strip_tags('<', '') . PHP_EOL; ?> --EXPECT-- :> :< +':> :< +:alert(0) +< \ No newline at end of file From 7b85f23d89091d63c7199a054ee3c81cc849a199 Mon Sep 17 00:00:00 2001 From: Henk te Sligte Date: Sun, 14 Oct 2018 20:42:59 +0200 Subject: [PATCH 3/3] Changed strip_tags function to escape < and > --- ext/filter/sanitizing_filters.c | 8 ++-- ext/standard/file.c | 13 +++--- ext/standard/php_string.h | 4 +- ext/standard/string.c | 57 ++++++++++++++++++++---- ext/standard/tests/strings/bug74371.phpt | 4 +- 5 files changed, 64 insertions(+), 22 deletions(-) diff --git a/ext/filter/sanitizing_filters.c b/ext/filter/sanitizing_filters.c index 7a992b4966c18..5dc09b28247dd 100644 --- a/ext/filter/sanitizing_filters.c +++ b/ext/filter/sanitizing_filters.c @@ -182,6 +182,7 @@ void php_filter_string(PHP_INPUT_FILTER_PARAM_DECL) { size_t new_len; unsigned char enc[256] = {0}; + zend_string* newval; if (!Z_REFCOUNTED_P(value)) { ZVAL_STRINGL(value, Z_STRVAL_P(value), Z_STRLEN_P(value)); @@ -206,10 +207,11 @@ void php_filter_string(PHP_INPUT_FILTER_PARAM_DECL) php_filter_encode_html(value, enc); /* strip tags, implicitly also removes \0 chars */ - new_len = php_strip_tags_ex(Z_STRVAL_P(value), Z_STRLEN_P(value), NULL, NULL, 0, 1); - Z_STRLEN_P(value) = new_len; + newval = php_strip_tags_ex(Z_STRVAL_P(value), Z_STRLEN_P(value), NULL, NULL, 0, 1); + zval_ptr_dtor(value); + ZVAL_STR(value, newval); - if (new_len == 0) { + if (ZSTR_LEN(newval) == 0) { zval_ptr_dtor(value); if (flags & FILTER_FLAG_EMPTY_STRING_NULL) { ZVAL_NULL(value); diff --git a/ext/standard/file.c b/ext/standard/file.c index 4fa498cdf4df6..fd6052cc239e4 100644 --- a/ext/standard/file.c +++ b/ext/standard/file.c @@ -1099,7 +1099,8 @@ PHPAPI PHP_FUNCTION(fgetss) zend_long bytes = 0; size_t len = 0; size_t actual_len, retval_len; - char *buf = NULL, *retval; + char *buf = NULL, *str; + zend_string* retval; php_stream *stream; char *allowed_tags=NULL; size_t allowed_tags_len=0; @@ -1125,18 +1126,16 @@ PHPAPI PHP_FUNCTION(fgetss) memset(buf, 0, len + 1); } - if ((retval = php_stream_get_line(stream, buf, len, &actual_len)) == NULL) { + if ((str = php_stream_get_line(stream, buf, len, &actual_len)) == NULL) { if (buf != NULL) { efree(buf); } RETURN_FALSE; } - retval_len = php_strip_tags(retval, actual_len, &stream->fgetss_state, allowed_tags, allowed_tags_len); - - // TODO: avoid reallocation ??? - RETVAL_STRINGL(retval, retval_len); - efree(retval); + retval = php_strip_tags(str, actual_len, &stream->fgetss_state, allowed_tags, allowed_tags_len); + efree(str); + RETURN_NEW_STR(retval); } /* }}} */ diff --git a/ext/standard/php_string.h b/ext/standard/php_string.h index 7e49b3ee3b7c0..cd84baaa8e5f2 100644 --- a/ext/standard/php_string.h +++ b/ext/standard/php_string.h @@ -140,8 +140,8 @@ PHPAPI char *php_stristr(char *s, char *t, size_t s_len, size_t t_len); PHPAPI zend_string *php_str_to_str(const char *haystack, size_t length, const char *needle, size_t needle_len, const char *str, size_t str_len); PHPAPI zend_string *php_trim(zend_string *str, char *what, size_t what_len, int mode); -PHPAPI size_t php_strip_tags(char *rbuf, size_t len, uint8_t *state, const char *allow, size_t allow_len); -PHPAPI size_t php_strip_tags_ex(char *rbuf, size_t len, uint8_t *stateptr, const char *allow, size_t allow_len, zend_bool allow_tag_spaces); +PHPAPI zend_string *php_strip_tags(char *rbuf, size_t len, uint8_t *state, const char *allow, size_t allow_len); +PHPAPI zend_string *php_strip_tags_ex(char *rbuf, size_t len, uint8_t *stateptr, const char *allow, size_t allow_len, zend_bool allow_tag_spaces); PHPAPI void php_implode(const zend_string *delim, zval *arr, zval *return_value); PHPAPI void php_explode(const zend_string *delim, zend_string *str, zval *return_value, zend_long limit); diff --git a/ext/standard/string.c b/ext/standard/string.c index 974ea35498472..500bc63437cef 100644 --- a/ext/standard/string.c +++ b/ext/standard/string.c @@ -4778,6 +4778,7 @@ PHP_FUNCTION(nl2br) PHP_FUNCTION(strip_tags) { zend_string *buf; + zend_string *newbuf; zend_string *str; zval *allow=NULL; const char *allowed_tags=NULL; @@ -4797,8 +4798,10 @@ PHP_FUNCTION(strip_tags) } buf = zend_string_init(ZSTR_VAL(str), ZSTR_LEN(str), 0); - ZSTR_LEN(buf) = php_strip_tags_ex(ZSTR_VAL(buf), ZSTR_LEN(str), NULL, allowed_tags, allowed_tags_len, 0); - RETURN_NEW_STR(buf); + newbuf = php_strip_tags_ex(ZSTR_VAL(buf), ZSTR_LEN(str), NULL, allowed_tags, allowed_tags_len, 0); + zend_string_release(buf); + + RETURN_NEW_STR(newbuf); } /* }}} */ @@ -5002,7 +5005,7 @@ int php_tag_find(char *tag, size_t len, const char *set) { } /* }}} */ -PHPAPI size_t php_strip_tags(char *rbuf, size_t len, uint8_t *stateptr, const char *allow, size_t allow_len) /* {{{ */ +PHPAPI zend_string *php_strip_tags(char *rbuf, size_t len, uint8_t *stateptr, const char *allow, size_t allow_len) /* {{{ */ { return php_strip_tags_ex(rbuf, len, stateptr, allow, allow_len, 0); } @@ -5028,9 +5031,9 @@ PHPAPI size_t php_strip_tags(char *rbuf, size_t len, uint8_t *stateptr, const ch swm: Added ability to strip (>) + pos = rp - rbuf; + rbuf = erealloc(rbuf, len += 3); // there was room for 1 <, add 3 more chars + rp = rbuf + pos; + + // resize temporary buffer if necessary + if ((tp - tbuf) + 4 >= PHP_TAG_BUF_SIZE) { + pos = tp - tbuf; + tbuf = erealloc(tbuf, pos + PHP_TAG_BUF_SIZE + 1); // add room for < + tp = tbuf + pos; + } + + // store escaped < + strcpy(tp, "<"); + tp += 4; + } if (in_q) { - goto reg_char_1; + break; } if (isspace(*(p + 1)) && !allow_tag_spaces) { goto reg_char_1; @@ -5135,8 +5155,25 @@ PHPAPI size_t php_strip_tags_ex(char *rbuf, size_t len, uint8_t *stateptr, const depth--; break; } + if (in_q && allow) { + // resize return buffer to store escaped > (>) + pos = rp - rbuf; + rbuf = erealloc(rbuf, len += 3); // there was room for 1 <, add 3 more chars + rp = rbuf + pos; + + // resize temporary buffer + if ((tp - tbuf) + 4 >= PHP_TAG_BUF_SIZE) { + pos = tp - tbuf; + tbuf = erealloc(tbuf, pos + PHP_TAG_BUF_SIZE + 1); // add room for > + tp = tbuf + pos; + } + + // store escaped > + strcpy(tp, ">"); + tp += 4; + } if (in_q) { - goto reg_char_1; + break; } lc = '>'; @@ -5196,6 +5233,7 @@ PHPAPI size_t php_strip_tags_ex(char *rbuf, size_t len, uint8_t *stateptr, const if (allow) { if (tp - tbuf >= PHP_TAG_BUF_SIZE) { pos = tp - tbuf; + tbuf = erealloc(tbuf, (tp - tbuf) + PHP_TAG_BUF_SIZE + 1); tp = tbuf + pos; } @@ -5355,7 +5393,10 @@ PHPAPI size_t php_strip_tags_ex(char *rbuf, size_t len, uint8_t *stateptr, const if (stateptr) *stateptr = state; - return (size_t)(rp - rbuf); + // copy string to new buffer + size_t new_len = (size_t)(rp-rbuf); + + return zend_string_init(rbuf, (size_t)(rp - rbuf), 0); } /* }}} */ diff --git a/ext/standard/tests/strings/bug74371.phpt b/ext/standard/tests/strings/bug74371.phpt index e97711a34ee27..f66a97731a8b0 100644 --- a/ext/standard/tests/strings/bug74371.phpt +++ b/ext/standard/tests/strings/bug74371.phpt @@ -10,7 +10,7 @@ echo strip_tags('<', '') . PHP_EOL; ?> --EXPECT-- -:> :< -':> :< +:> :< +':> :< :alert(0) < \ No newline at end of file