[CVE-2017-11836] Edge - Assertion only bound check can lead to user controlled allocation size and write - Individual

When parsing a string template literal, we temporarily suspend updating some of the offset counters. If an attacker crafts a string template that contains multibyte characters, it is possible to overflow the minLine counter. The attacker can directly control the size of the overflow, which is then used in an allocation and offset calculation when handling a parse error. Further control can be gained by triggering a scanner capture and restore to propagate this bad counter value.
Unfortunately this would have been caught by existing bounds checks, but they are asserts only. There are other instances of these around the codebase, but I did not see any exploits for those.
To fix this, we manually update the minLine counter at the point where it would normally have been updated outside of a string template. I also changed the subtraction overflow asserts to be failfasts and added an assert for detecting overflow with multibyte characters. In the non-assert case, we will return 0. Other similar checks in IchMinTok and IchLimTok have also been converted.

Author: tcare

lib/Parser/Scan.cpp

@@ -1110,6 +1110,10 @@ tokens Scanner<EncodingPolicy>::ScanStringConstant(OLECHAR delim, EncodedCharPtr
             {
                 // Notify the scanner to update current line, number of lines etc
                 NotifyScannedNewLine();
+
+                // We haven't updated m_currentCharacter yet, so make sure the MinLine info is correct in case we error out.
+                m_pchMinLine = p;
+
                 break;
             }
 
@@ -1400,6 +1404,10 @@ tokens Scanner<EncodingPolicy>::ScanStringConstant(OLECHAR delim, EncodedCharPtr
 
                     // Template literal strings ignore all escaped line continuation tokens
                     NotifyScannedNewLine();
+
+                    // We haven't updated m_currentCharacter yet, so make sure the MinLine info is correct in case we error out.
+                    m_pchMinLine = p;
+
                     continue;
                 }
 

lib/Parser/Scan.h

@@ -480,17 +480,31 @@ class Scanner : public IScanner, public EncodingPolicy
     // have if the entire file was converted to Unicode (UTF16-LE).
     charcount_t IchMinTok(void) const
     {
-        Assert(m_pchMinTok - m_pchBase >= 0);
-        Assert(m_pchMinTok - m_pchBase <= LONG_MAX);
-        return static_cast< charcount_t >(m_pchMinTok - m_pchBase - m_cMinTokMultiUnits);
+
+        AssertOrFailFast(m_pchMinTok - m_pchBase >= 0);
+        AssertOrFailFast(m_pchMinTok - m_pchBase <= LONG_MAX);
+        if (static_cast<charcount_t>(m_pchMinTok - m_pchBase) < m_cMinTokMultiUnits)
+        {
+            AssertMsg(false, "IchMinTok subtraction overflow");
+            return 0;
+        }
+
+        return static_cast<charcount_t>(m_pchMinTok - m_pchBase - m_cMinTokMultiUnits);
     }
 
     // Returns the character offset of the character immediately following the token. The character offset is the offset the first
     // character of the token would have if the entire file was converted to Unicode (UTF16-LE).
     charcount_t IchLimTok(void) const
     {
-        Assert(m_currentCharacter - m_pchBase >= 0);
-        Assert(m_currentCharacter - m_pchBase <= LONG_MAX);
+
+        AssertOrFailFast(m_currentCharacter - m_pchBase >= 0);
+        AssertOrFailFast(m_currentCharacter - m_pchBase <= LONG_MAX);
+        if (static_cast<charcount_t>(m_currentCharacter - m_pchBase) < this->m_cMultiUnits)
+        {
+            AssertMsg(false, "IchLimTok subtraction overflow");
+            return 0;
+        }
+
         return static_cast< charcount_t >(m_currentCharacter - m_pchBase - this->m_cMultiUnits);
     }
 
@@ -542,8 +556,15 @@ class Scanner : public IScanner, public EncodingPolicy
     // Returns the character offset within the stream of the first character on the current line.
     charcount_t IchMinLine(void) const
     {
-        Assert(m_pchMinLine - m_pchBase >= 0);
-        Assert(m_pchMinLine - m_pchBase <= LONG_MAX);
+
+        AssertOrFailFast(m_pchMinLine - m_pchBase >= 0);
+        AssertOrFailFast(m_pchMinLine - m_pchBase <= LONG_MAX);
+        if (static_cast<charcount_t>(m_pchMinLine - m_pchBase) < m_cMinLineMultiUnits)
+        {
+            AssertMsg(false, "IchMinLine subtraction overflow");
+            return 0;
+        }
+
         return static_cast<charcount_t>(m_pchMinLine - m_pchBase - m_cMinLineMultiUnits);
     }