Skip to content

Commit aa24abd

Browse files
dmsnellsirrealockham
committed
HTML API: Prevent out-of-bounds string access.
Rebuild of 8905faf after WordPress#5725. Co-authored-by: Jon Surrell <sirreal@users.noreply.github.com> Co-authored-by: Bernie Reiter <bernhard-reiter@git.wordpress.org>
1 parent c3166be commit aa24abd

File tree

2 files changed

+31
-8
lines changed

2 files changed

+31
-8
lines changed

src/wp-includes/html-api/class-wp-html-tag-processor.php

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,7 +1312,7 @@ private function parse_next_tag() {
13121312

13131313
$this->token_starts_at = $at;
13141314

1315-
if ( '/' === $this->html[ $at + 1 ] ) {
1315+
if ( $at + 1 < $doc_length && '/' === $this->html[ $at + 1 ] ) {
13161316
$this->is_closing_tag = true;
13171317
++$at;
13181318
} else {
@@ -1346,7 +1346,7 @@ private function parse_next_tag() {
13461346
* Abort if no tag is found before the end of
13471347
* the document. There is nothing left to parse.
13481348
*/
1349-
if ( $at + 1 >= strlen( $html ) ) {
1349+
if ( $at + 1 >= $doc_length ) {
13501350
$this->parser_state = self::STATE_INCOMPLETE;
13511351

13521352
return false;
@@ -1368,7 +1368,7 @@ private function parse_next_tag() {
13681368
) {
13691369
$closer_at = $at + 4;
13701370
// If it's not possible to close the comment then there is nothing more to scan.
1371-
if ( strlen( $html ) <= $closer_at ) {
1371+
if ( $doc_length <= $closer_at ) {
13721372
$this->parser_state = self::STATE_INCOMPLETE;
13731373

13741374
return false;
@@ -1388,20 +1388,20 @@ private function parse_next_tag() {
13881388
* See https://html.spec.whatwg.org/#parse-error-incorrectly-closed-comment
13891389
*/
13901390
--$closer_at; // Pre-increment inside condition below reduces risk of accidental infinite looping.
1391-
while ( ++$closer_at < strlen( $html ) ) {
1391+
while ( ++$closer_at < $doc_length ) {
13921392
$closer_at = strpos( $html, '--', $closer_at );
13931393
if ( false === $closer_at ) {
13941394
$this->parser_state = self::STATE_INCOMPLETE;
13951395

13961396
return false;
13971397
}
13981398

1399-
if ( $closer_at + 2 < strlen( $html ) && '>' === $html[ $closer_at + 2 ] ) {
1399+
if ( $closer_at + 2 < $doc_length && '>' === $html[ $closer_at + 2 ] ) {
14001400
$at = $closer_at + 3;
14011401
continue 2;
14021402
}
14031403

1404-
if ( $closer_at + 3 < strlen( $html ) && '!' === $html[ $closer_at + 2 ] && '>' === $html[ $closer_at + 3 ] ) {
1404+
if ( $closer_at + 3 < $doc_length && '!' === $html[ $closer_at + 2 ] && '>' === $html[ $closer_at + 3 ] ) {
14051405
$at = $closer_at + 4;
14061406
continue 2;
14071407
}
@@ -1414,7 +1414,7 @@ private function parse_next_tag() {
14141414
* https://html.spec.whatwg.org/multipage/parsing.html#tag-open-state
14151415
*/
14161416
if (
1417-
strlen( $html ) > $at + 8 &&
1417+
$doc_length > $at + 8 &&
14181418
'[' === $html[ $at + 2 ] &&
14191419
'C' === $html[ $at + 3 ] &&
14201420
'D' === $html[ $at + 4 ] &&
@@ -1440,7 +1440,7 @@ private function parse_next_tag() {
14401440
* https://html.spec.whatwg.org/multipage/parsing.html#tag-open-state
14411441
*/
14421442
if (
1443-
strlen( $html ) > $at + 8 &&
1443+
$doc_length > $at + 8 &&
14441444
( 'D' === $html[ $at + 2 ] || 'd' === $html[ $at + 2 ] ) &&
14451445
( 'O' === $html[ $at + 3 ] || 'o' === $html[ $at + 3 ] ) &&
14461446
( 'C' === $html[ $at + 4 ] || 'c' === $html[ $at + 4 ] ) &&
@@ -1512,6 +1512,11 @@ private function parse_next_tag() {
15121512
* See https://html.spec.whatwg.org/#parse-error-invalid-first-character-of-tag-name
15131513
*/
15141514
if ( $this->is_closing_tag ) {
1515+
// No chance of finding a closer.
1516+
if ( $at + 3 > $doc_length ) {
1517+
return false;
1518+
}
1519+
15151520
$closer_at = strpos( $html, '>', $at + 3 );
15161521
if ( false === $closer_at ) {
15171522
$this->parser_state = self::STATE_INCOMPLETE;

tests/phpunit/tests/html-api/wpHtmlTagProcessor.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2691,4 +2691,22 @@ public function data_updating_attributes_in_malformed_html() {
26912691
),
26922692
);
26932693
}
2694+
2695+
/**
2696+
* @covers WP_HTML_Tag_Processor::next_tag
2697+
*/
2698+
public function test_handles_malformed_taglike_open_short_html() {
2699+
$p = new WP_HTML_Tag_Processor( '<' );
2700+
$result = $p->next_tag();
2701+
$this->assertFalse( $result, 'Did not handle "<" html properly.' );
2702+
}
2703+
2704+
/**
2705+
* @covers WP_HTML_Tag_Processor::next_tag
2706+
*/
2707+
public function test_handles_malformed_taglike_close_short_html() {
2708+
$p = new WP_HTML_Tag_Processor( '</ ' );
2709+
$result = $p->next_tag();
2710+
$this->assertFalse( $result, 'Did not handle "</ " html properly.' );
2711+
}
26942712
}

0 commit comments

Comments
 (0)