JavaScriptMinifier: Fix "Uninitialized offset" in string and regexp parsing
authorTimo Tijhof <krinklemail@gmail.com>
Wed, 10 Jan 2018 02:02:27 +0000 (02:02 +0000)
committerKrinkle <krinklemail@gmail.com>
Thu, 11 Jan 2018 17:47:59 +0000 (17:47 +0000)
When parsing an incomplete string literal or regex literal at the end of a file,
$end would be set to an offset higher than $length, because the code
speculatively increases $end without correcting for this scenario.

This is due to the assumption that the strcspn() search will end because
an end character was seen, instead of simply ending because the string
doesn't have any further characters.

Bug: T75556
Change-Id: I2325c9aff33293c13ff414699c2d47306182aaa6

includes/libs/JavaScriptMinifier.php
tests/phpunit/includes/libs/JavaScriptMinifierTest.php

index 679da2b..5e7373e 100644 (file)
@@ -455,6 +455,12 @@ class JavaScriptMinifier {
                                } while( $end - 2 < $length && $s[$end - 2] === '\\' );
                                // Correction (1): Undo speculative add, keep only one (end of string literal)
                                $end--;
+                               if ( $end > $length ) {
+                                       // Correction (2): Loop wrongly assumed an end quote ended the search,
+                                       // but search ended because we've reached the end. Correct $end.
+                                       // TODO: This is invalid and should throw.
+                                       $end--;
+                               }
                        // We have to distinguish between regexp literals and division operators
                        // A division operator is only possible in certain states
                        } elseif( $ch === '/' && !isset( $divStates[$state] ) ) {
@@ -471,8 +477,14 @@ class JavaScriptMinifier {
                                        } while( $end - 2 < $length && $s[$end - 2] === '\\' );
                                        // Correction (1): Undo speculative add, keep only one (end of regexp)
                                        $end--;
-                                       // If the end, stop here.
-                                       if( $end - 1 >= $length || $s[$end - 1] === '/' ) {
+                                       if ( $end > $length ) {
+                                               // Correction (2): Loop wrongly assumed end slash was seen
+                                               // String ended without end of regexp. Correct $end.
+                                               // TODO: This is invalid and should throw.
+                                               $end--;
+                                               break;
+                                       }
+                                       if ( $s[$end - 1] === '/' ) {
                                                break;
                                        }
                                        // (Implicit else), we must've found the start of a char class,
index 5061e27..d6a1040 100644 (file)
@@ -82,6 +82,14 @@ class JavaScriptMinifierTest extends PHPUnit_Framework_TestCase {
                                "var a=this\nfor(b=0;c<d;b++){}"
                        ],
 
+                       // Cover failure case of incomplete regexp at end of file (T75556)
+                       // FIXME: This is invalid, but currently tolerated
+                       [ "*/", "*/", false ],
+
+                       // Cover failure case of incomplete string at end of file (T75556)
+                       // FIXME: This is invalid, but currently tolerated
+                       [ "'a", "'a", false ],
+
                        // Token separation
                        [ "x  in  y", "x in y" ],
                        [ "/x/g  in  y", "/x/g in y" ],