[PHP-CVS] svn: /php/php-src/branches/PHP_5_4/ NEWS ext/standard/html.c ext/standard/tests/strings/bug60965.phpt
cataphract Sun, 05 Feb 2012 09:59:33 +
Revision: http://svn.php.net/viewvc?view=revisionrevision=323074
Log:
- Merge r323056 (see bug #60965).
Bug: https://bugs.php.net/60965 (Critical) Buffer overflow on
htmlspecialchars/entities with $double=false
Changed paths:
U php/php-src/branches/PHP_5_4/NEWS
U php/php-src/branches/PHP_5_4/ext/standard/html.c
A + php/php-src/branches/PHP_5_4/ext/standard/tests/strings/bug60965.phpt
(from
php/php-src/trunk/ext/standard/tests/strings/bug60965.phpt:r323056)
Modified: php/php-src/branches/PHP_5_4/NEWS
===
--- php/php-src/branches/PHP_5_4/NEWS 2012-02-05 09:58:50 UTC (rev 323073)
+++ php/php-src/branches/PHP_5_4/NEWS 2012-02-05 09:59:33 UTC (rev 323074)
@@ -1,10 +1,13 @@
PHPNEWS
|||
?? Feb 2012, PHP 5.4.0 RC 8
+- Core:
+ . Fixed bug #60965 (Buffer overflow on htmlspecialchars/entities with
+$double=false). (Gustavo)
02 Feb 2012, PHP 5.4.0 RC 7
- Core:
- . Fix bug #60895 (Possible invalid handler usage in windows random
+ . Fixed bug #60895 (Possible invalid handler usage in windows random
functions). (Pierre)
. Fixed bug #51860 (Include fails with toplevel symlink to /). (Dmitry)
. Fixed (disabled) inline-caching for ZEND_OVERLOADED_FUNCTION methods.
Modified: php/php-src/branches/PHP_5_4/ext/standard/html.c
===
--- php/php-src/branches/PHP_5_4/ext/standard/html.c2012-02-05 09:58:50 UTC
(rev 323073)
+++ php/php-src/branches/PHP_5_4/ext/standard/html.c2012-02-05 09:59:33 UTC
(rev 323074)
@@ -1215,7 +1215,6 @@
size_t cursor, maxlen, len;
char *replaced;
enum entity_charset charset = determine_charset(hint_charset TSRMLS_CC);
- int matches_map;
int doctype = flags ENT_HTML_DOC_TYPE_MASK;
entity_table_opt entity_table;
const enc_to_uni *to_uni_table = NULL;
@@ -1253,12 +1252,14 @@
}
}
+ /* initial estimate */
if (oldlen 64) {
maxlen = 128;
} else {
maxlen = 2 * oldlen;
}
- replaced = emalloc(maxlen);
+
+ replaced = emalloc(maxlen + 1);
len = 0;
cursor = 0;
while (cursor oldlen) {
@@ -1271,7 +1272,7 @@
/* guarantee we have at least 40 bytes to write.
* In HTML5, entities may take up to 33 bytes */
if (len + 40 maxlen) {
- replaced = erealloc(replaced, maxlen += 128);
+ replaced = erealloc(replaced, (maxlen += 128) + 1);
}
if (status == FAILURE) {
@@ -1291,7 +1292,6 @@
mbsequence = old[cursor_before];
mbseqlen = cursor - cursor_before;
}
- matches_map = 0;
if (this_char != '') { /* no entity on this position */
const unsigned char *rep= NULL;
@@ -1302,12 +1302,15 @@
goto pass_char_through;
if (all) { /* false that
CHARSET_PARTIAL_SUPPORT(charset) */
- /* look for entity for this char */
if (to_uni_table != NULL) {
+ /* !CHARSET_UNICODE_COMPAT therefore
not UTF-8; since UTF-8
+* is the only multibyte encoding with
!CHARSET_PARTIAL_SUPPORT,
+* we're using a single byte encoding */
map_to_unicode(this_char, to_uni_table,
this_char);
if (this_char == 0x) /* no mapping;
pass through */
goto pass_char_through;
}
+ /* the cursor may advance */
find_entity_for_char(this_char, charset,
entity_table.ms_table, rep,
rep_len, old, oldlen, cursor);
} else {
@@ -1397,6 +1400,10 @@
}
}
/* checks passed; copy entity to result */
+ /* entity size is unbounded, we may need more
memory */
+ if (maxlen len + ent_len + 2 /* and ; */) {
+ replaced = erealloc(replaced, (maxlen
+= ent_len + 128) + 1);
+ }
replaced[len++] = '';
memcpy(replaced[len], old[cursor], ent_len);
Re: [PHP-CVS] svn: /php/php-src/branches/PHP_5_4/ NEWS ext/standard/html.c ext/standard/tests/strings/bug60965.phpt
I didn't carefully review this patch, but doesn't this code suffer from
potential math overflow?
i.e. with strlen($input_str) INT_MAX/2 (or UINT_MAX/2)
Nuno
- Original Message -
From: Gustavo André dos Santos Lopes cataphr...@php.net
To: php-cvs@lists.php.net
Sent: Sunday, February 05, 2012 9:59 AM
Subject: [PHP-CVS] svn: /php/php-src/branches/PHP_5_4/ NEWS
ext/standard/html.c ext/standard/tests/strings/bug60965.phpt
cataphract Sun, 05 Feb 2012 09:59:33 +
Revision: http://svn.php.net/viewvc?view=revisionrevision=323074
Log:
- Merge r323056 (see bug #60965).
Bug: https://bugs.php.net/60965 (Critical) Buffer overflow on
htmlspecialchars/entities with $double=false
Changed paths:
U php/php-src/branches/PHP_5_4/NEWS
U php/php-src/branches/PHP_5_4/ext/standard/html.c
A +
php/php-src/branches/PHP_5_4/ext/standard/tests/strings/bug60965.phpt
(from
php/php-src/trunk/ext/standard/tests/strings/bug60965.phpt:r323056)
Modified: php/php-src/branches/PHP_5_4/NEWS
===
--- php/php-src/branches/PHP_5_4/NEWS 2012-02-05 09:58:50 UTC (rev 323073)
+++ php/php-src/branches/PHP_5_4/NEWS 2012-02-05 09:59:33 UTC (rev 323074)
@@ -1,10 +1,13 @@
PHP
NEWS
|||
?? Feb 2012, PHP 5.4.0 RC 8
+- Core:
+ . Fixed bug #60965 (Buffer overflow on htmlspecialchars/entities with
+$double=false). (Gustavo)
02 Feb 2012, PHP 5.4.0 RC 7
- Core:
- . Fix bug #60895 (Possible invalid handler usage in windows random
+ . Fixed bug #60895 (Possible invalid handler usage in windows random
functions). (Pierre)
. Fixed bug #51860 (Include fails with toplevel symlink to /). (Dmitry)
. Fixed (disabled) inline-caching for ZEND_OVERLOADED_FUNCTION methods.
Modified: php/php-src/branches/PHP_5_4/ext/standard/html.c
===
--- php/php-src/branches/PHP_5_4/ext/standard/html.c 2012-02-05 09:58:50
UTC (rev 323073)
+++ php/php-src/branches/PHP_5_4/ext/standard/html.c 2012-02-05 09:59:33
UTC (rev 323074)
@@ -1215,7 +1215,6 @@
size_t cursor, maxlen, len;
char *replaced;
enum entity_charset charset = determine_charset(hint_charset TSRMLS_CC);
- int matches_map;
int doctype = flags ENT_HTML_DOC_TYPE_MASK;
entity_table_opt entity_table;
const enc_to_uni *to_uni_table = NULL;
@@ -1253,12 +1252,14 @@
}
}
+ /* initial estimate */
if (oldlen 64) {
maxlen = 128;
} else {
maxlen = 2 * oldlen;
}
- replaced = emalloc(maxlen);
+
+ replaced = emalloc(maxlen + 1);
len = 0;
cursor = 0;
while (cursor oldlen) {
@@ -1271,7 +1272,7 @@
/* guarantee we have at least 40 bytes to write.
* In HTML5, entities may take up to 33 bytes */
if (len + 40 maxlen) {
- replaced = erealloc(replaced, maxlen += 128);
+ replaced = erealloc(replaced, (maxlen += 128) + 1);
}
if (status == FAILURE) {
@@ -1291,7 +1292,6 @@
mbsequence = old[cursor_before];
mbseqlen = cursor - cursor_before;
}
- matches_map = 0;
if (this_char != '') { /* no entity on this position */
const unsigned char *rep = NULL;
@@ -1302,12 +1302,15 @@
goto pass_char_through;
if (all) { /* false that CHARSET_PARTIAL_SUPPORT(charset) */
- /* look for entity for this char */
if (to_uni_table != NULL) {
+ /* !CHARSET_UNICODE_COMPAT therefore not UTF-8; since UTF-8
+ * is the only multibyte encoding with !CHARSET_PARTIAL_SUPPORT,
+ * we're using a single byte encoding */
map_to_unicode(this_char, to_uni_table, this_char);
if (this_char == 0x) /* no mapping; pass through */
goto pass_char_through;
}
+ /* the cursor may advance */
find_entity_for_char(this_char, charset, entity_table.ms_table, rep,
rep_len, old, oldlen, cursor);
} else {
@@ -1397,6 +1400,10 @@
}
}
/* checks passed; copy entity to result */
+ /* entity size is unbounded, we may need more memory */
+ if (maxlen len + ent_len + 2 /* and ; */) {
+ replaced = erealloc(replaced, (maxlen += ent_len + 128) + 1);
+ }
replaced[len++] = '';
memcpy(replaced[len], old[cursor], ent_len);
len += ent_len;
Copied:
php/php-src/branches/PHP_5_4/ext/standard/tests/strings/bug60965.phpt
(from rev 323056,
php/php-src/trunk/ext/standard/tests/strings/bug60965.phpt)
===
--- php/php-src/branches/PHP_5_4/ext/standard/tests/strings/bug60965.phpt
(rev 0)
+++ php/php-src/branches/PHP_5_4/ext/standard/tests/strings/bug60965.phpt
2012-02-05 09:59:33 UTC (rev 323074)
@@ -0,0 +1,10 @@
+--TEST--
+Bug #60965: Buffer overflow on htmlspecialchars/entities with
$double=false
+--FILE--
+?php
+echo
htmlspecialchars('#x05;',
+ENT_QUOTES, 'UTF-8', false), \n;
+echo Done.\n;
+--EXPECT--
+quot;quot;quot;quot;quot;quot;quot
Re: [PHP-CVS] svn: /php/php-src/branches/PHP_5_4/ NEWS ext/standard/html.c ext/standard/tests/strings/bug60965.phpt
On Sun, 5 Feb 2012 10:55:39 -, Nuno Lopes wrote: I didn't carefully review this patch, but doesn't this code suffer from potential math overflow? i.e. with strlen($input_str) INT_MAX/2 (or UINT_MAX/2) All the length and position variables are of type size_t, so I'd say we'd be out of memory long before that could be a problem (unless there's some architecture of which I'm not aware where SIZE_T is low enough for this to be a problem). -- Gustavo Lopes -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] svn: /php/php-src/branches/PHP_5_4/ NEWS ext/standard/html.c ext/standard/tests/strings/bug60965.phpt
On Sun, 05 Feb 2012 14:00:11 +0100, Gustavo Lopes wrote: On Sun, 5 Feb 2012 10:55:39 -, Nuno Lopes wrote: I didn't carefully review this patch, but doesn't this code suffer from potential math overflow? i.e. with strlen($input_str) INT_MAX/2 (or UINT_MAX/2) All the length and position variables are of type size_t, so I'd say we'd be out of memory long before that could be a problem (unless there's some architecture of which I'm not aware where SIZE_T is low enough for this to be a problem). read: SIZE_MAX, not SIZE_T -- Gustavo Lopes -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] svn: /php/php-src/branches/PHP_5_4/ NEWS ext/standard/html.c ext/standard/tests/strings/bug60965.phpt
2012/2/5 Gustavo Lopes glo...@nebm.ist.utl.pt: All the length and position variables are of type size_t, so I'd say we'd be out of memory long before that could be a problem (unless there's some architecture of which I'm not aware where SIZE_T is low enough for this to be a problem). read: SIZE_MAX, not SIZE_T By the way, SIZE_MAX (can be up to 65k or so afair) should not be used in relation with buffer (string or other) length. It defines the maximum size of a single object allocation that the compiler can manage. Not sure if it is actually what you want here. -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] svn: /php/php-src/branches/PHP_5_4/ NEWS ext/standard/html.c ext/standard/tests/strings/bug60965.phpt
On Sun, 5 Feb 2012 14:37:27 +0100, Pierre Joye wrote: 2012/2/5 Gustavo Lopes glo...@nebm.ist.utl.pt: All the length and position variables are of type size_t, so I'd say we'd be out of memory long before that could be a problem (unless there's some architecture of which I'm not aware where SIZE_T is low enough for this to be a problem). read: SIZE_MAX, not SIZE_T By the way, SIZE_MAX (can be up to 65k or so afair) should not be used in relation with buffer (string or other) length. It defines the maximum size of a single object allocation that the compiler can manage. Not sure if it is actually what you want here. SIZE_MAX is indeed the limit of size_t. See ISO/IEC 9899:TC3, section 7.18.3 on http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf (page 259). Forgetting the irrelevant case where size_t is 16 bit wide, there is indeed a potential problem if size_t is 32-bit wide. First, if you can pass a string with about 2GB you could the multiplication by 2 would wrap around. But you could even pass a smaller string (possibly 10/15 times less, I don't know what's the maximum expansion factor of htmlentities) and then it could wrap in the reallocation. I'll take this into account. -- Gustavo Lopes -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] svn: /php/php-src/branches/PHP_5_4/ NEWS ext/standard/html.c ext/standard/tests/strings/bug60965.phpt
On Sun, 05 Feb 2012 15:00:11 +0100, Gustavo Lopes wrote: On Sun, 5 Feb 2012 14:37:27 +0100, Pierre Joye wrote: 2012/2/5 Gustavo Lopes glo...@nebm.ist.utl.pt: All the length and position variables are of type size_t, so I'd say we'd be out of memory long before that could be a problem (unless there's some architecture of which I'm not aware where SIZE_T is low enough for this to be a problem). read: SIZE_MAX, not SIZE_T By the way, SIZE_MAX (can be up to 65k or so afair) should not be used in relation with buffer (string or other) length. It defines the maximum size of a single object allocation that the compiler can manage. Not sure if it is actually what you want here. SIZE_MAX is indeed the limit of size_t. See ISO/IEC 9899:TC3, section 7.18.3 on http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf (page 259). Forgetting the irrelevant case where size_t is 16 bit wide, there is indeed a potential problem if size_t is 32-bit wide. First, if you can pass a string with about 2GB you could the multiplication by 2 would wrap around. But you could even pass a smaller string (possibly 10/15 times less, I don't know what's the maximum expansion factor of htmlentities) and then it could wrap in the reallocation. I'll take this into account. See http://svn.php.net/viewvc/php/php-src/trunk/ext/standard/html.c?r1=323079r2=323078pathrev=323079 I don't know if this is worth merging to 5.4 at this point; after all 5.3 has the same problem. -- Gustavo Lopes -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] svn: /php/php-src/branches/PHP_5_4/ NEWS ext/standard/html.c ext/standard/tests/strings/bug60965.phpt
On Sun, 05 Feb 2012 15:00:11 +0100, Gustavo Lopes wrote: On Sun, 5 Feb 2012 14:37:27 +0100, Pierre Joye wrote: 2012/2/5 Gustavo Lopes glo...@nebm.ist.utl.pt: All the length and position variables are of type size_t, so I'd say we'd be out of memory long before that could be a problem (unless there's some architecture of which I'm not aware where SIZE_T is low enough for this to be a problem). read: SIZE_MAX, not SIZE_T By the way, SIZE_MAX (can be up to 65k or so afair) should not be used in relation with buffer (string or other) length. It defines the maximum size of a single object allocation that the compiler can manage. Not sure if it is actually what you want here. SIZE_MAX is indeed the limit of size_t. See ISO/IEC 9899:TC3, section 7.18.3 on http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf (page 259). Forgetting the irrelevant case where size_t is 16 bit wide, there is indeed a potential problem if size_t is 32-bit wide. First, if you can pass a string with about 2GB you could the multiplication by 2 would wrap around. But you could even pass a smaller string (possibly 10/15 times less, I don't know what's the maximum expansion factor of htmlentities) and then it could wrap in the reallocation. I'll take this into account. See http://svn.php.net/viewvc/php/php-src/trunk/ext/standard/html.c?r1=323079r2=323078pathrev=323079 I don't know if this is worth merging to 5.4 at this point; after all 5.3 has the same problem. Obrigado! I think this bug (although probably exploitable) is low risk, since it requires a large 'memory_limit' value to be triggable. Your last patch seems good to me. Nuno -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
