Teach the background parser to ignore certain elements inside '<select>'.

third_party/WebKit/Source/core/html/parser/HTMLTreeBuilderSimulator.cpp

Index: third_party/WebKit/Source/core/html/parser/HTMLTreeBuilderSimulator.cpp
diff --git a/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilderSimulator.cpp b/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilderSimulator.cpp
index d23ba6139b787362ccc9bbd6358ba352763fd952..aa8d08149e511636494fbb7a25fedb19f7fcaa88 100644
--- a/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilderSimulator.cpp
+++ b/third_party/WebKit/Source/core/html/parser/HTMLTreeBuilderSimulator.cpp
@@ -92,9 +92,17 @@ static bool tokenExitsMath(const CompactHTMLToken& token) {
          threadSafeMatch(tagName, MathMLNames::mtextTag);
 }
 
+static bool tokenExitsInSelect(const CompactHTMLToken& token) {
+  // https://html.spec.whatwg.org/#parsing-main-inselect
+  const String& tagName = token.data();
+  return threadSafeMatch(tagName, inputTag) ||
+         threadSafeMatch(tagName, keygenTag) ||
+         threadSafeMatch(tagName, textareaTag);
+}
+
 HTMLTreeBuilderSimulator::HTMLTreeBuilderSimulator(
     const HTMLParserOptions& options)
-    : m_options(options) {
+    : m_options(options), m_inSelectInsertionMode(false) {
   m_namespaceStack.push_back(HTML);
 }
 
@@ -140,20 +148,38 @@ HTMLTreeBuilderSimulator::SimulatedToken HTMLTreeBuilderSimulator::simulate(
       if (threadSafeMatch(tagName, textareaTag) ||
           threadSafeMatch(tagName, titleTag)) {
         tokenizer->setState(HTMLTokenizer::RCDATAState);
-      } else if (threadSafeMatch(tagName, plaintextTag)) {
-        tokenizer->setState(HTMLTokenizer::PLAINTEXTState);
       } else if (threadSafeMatch(tagName, scriptTag)) {
         tokenizer->setState(HTMLTokenizer::ScriptDataState);
         simulatedToken = ScriptStart;
-      } else if (threadSafeMatch(tagName, styleTag) ||
-                 threadSafeMatch(tagName, iframeTag) ||
-                 threadSafeMatch(tagName, xmpTag) ||
-                 (threadSafeMatch(tagName, noembedTag) &&
-                  m_options.pluginsEnabled) ||
-                 threadSafeMatch(tagName, noframesTag) ||
-                 (threadSafeMatch(tagName, noscriptTag) &&
-                  m_options.scriptEnabled)) {
-        tokenizer->setState(HTMLTokenizer::RAWTEXTState);
+      } else if (!m_inSelectInsertionMode) {
+        // If we're in the "in select" insertion mode, all of these tags are
+        // ignored, so we shouldn't change the tokenizer state:
+        // https://html.spec.whatwg.org/#parsing-main-inselect
+        if (threadSafeMatch(tagName, plaintextTag) &&
+            !m_inSelectInsertionMode) {

[Yoav Weiss, 2017/01/12 13:45:44]

I'm most probably missing something, but I don't see such a condition in the
HTML spec. What I do see is that moving the tokenizer to the PLAINTEXTState is
defined to only happen when parsing *in body*
https://html.spec.whatwg.org/#parsing-main-inbody

So IIUC (and it's fairly possible that this is not the case) this should be
blocked on in-body insertion, not just banned inside select.

[Mike West, 2017/01/12 14:44:51]

Sure. However, we have no idea what mode we're in in this simulated tree
builder. I agree with you that this patch leaves correctness problems with
regard to switching into plaintext or rawtext tokenizing. From my perspective, I
don't particularly care about those correctness problems, because they don't
also turn into security problems.

Following those rules in a reasonable way almost certainly boils down to
implementing all the logic in HTMLTreeBuilder about insertion mode state
transitions that we're leaving out. I'd be thrilled if y'all would take that on,
because the disconnect between the tokenizer state and the insertion mode seems
very strange to me, and there are probably other problems that it causes. I'm
not signing up to do that work, though. :)

[Yoav Weiss, 2017/01/13 14:13:19]

makes sense :)

+          tokenizer->setState(HTMLTokenizer::PLAINTEXTState);
+        } else if (threadSafeMatch(tagName, styleTag) ||
+                   threadSafeMatch(tagName, iframeTag) ||
+                   threadSafeMatch(tagName, xmpTag) ||
+                   (threadSafeMatch(tagName, noembedTag) &&
+                    m_options.pluginsEnabled) ||
+                   threadSafeMatch(tagName, noframesTag) ||
+                   (threadSafeMatch(tagName, noscriptTag) &&
+                    m_options.scriptEnabled)) {

[Yoav Weiss, 2017/01/12 13:45:44]

Here we're basically blocking anything not mentioned in the "in select insertion
mode" rather than allowing what's there? Is there a particular reason for that?
Can we inverse it to make it easier to correlate the spec to the code and vice
versa?

[Mike West, 2017/01/12 14:44:51]

No? This basically just moves the existing code (from line 143 and line 148)
into an `if (!m_inSelectInsertionMode)` block, ensuring that we don't transition
the tokenizer state into PLAINTEXTState or RAWTEXTState if we're in "in select"
insertion mode.

Again, the actual work on the tokens (e.g. whether or not the `<plaintext>`
start tag is ignored or not) is handled in HTMLTreeBuilder. That's where the
insertion mode matters and is stored. I'm only porting over the smallest piece
necessary to teach this simulated treebuilder enough to stop it from changing
the tokenizer state in this particular case with security implications. :)

[Yoav Weiss, 2017/01/13 14:13:19]

fair enough :)

+          tokenizer->setState(HTMLTokenizer::RAWTEXTState);
+        }
+      }
+
+      // We need to track whether we're in the "in select" insertion mode
+      // in order to determine whether '<plaintext>' will put the tokenizer
+      // into PLAINTEXTState, and whether '<xmp>' and others will consume
+      // textual content.
+      //
+      // https://html.spec.whatwg.org/#parsing-main-inselect
+      if (threadSafeMatch(tagName, selectTag)) {
+        m_inSelectInsertionMode = true;
+      } else if (m_inSelectInsertionMode && tokenExitsInSelect(token)) {
+        m_inSelectInsertionMode = false;
       }
     }
   }
@@ -169,12 +195,15 @@ HTMLTreeBuilderSimulator::SimulatedToken HTMLTreeBuilderSimulator::simulate(
         (m_namespaceStack.contains(SVG) && m_namespaceStack.back() == HTML &&
          tokenExitsSVG(token)) ||
         (m_namespaceStack.contains(MathML) && m_namespaceStack.back() == HTML &&
-         tokenExitsMath(token)))
+         tokenExitsMath(token))) {
       m_namespaceStack.pop_back();
+    }
     if (threadSafeMatch(tagName, scriptTag)) {
       if (!inForeignContent())
         tokenizer->setState(HTMLTokenizer::DataState);
       return ScriptEnd;
+    } else if (threadSafeMatch(tagName, selectTag)) {
+      m_inSelectInsertionMode = false;
     }
   }