CVE-2021-33194: Loop with Unreachable Exit Condition ('Infinite Loop') (CWE-835)
value: golang.org/x/net before v0.0.0-20210520170846-37e1c6afe023 allows attackers to cause a denial of service (infinite loop) via crafted ParseFragment input.
CWE description: The product contains an iteration or loop with an exit condition that cannot be reached, i.e., an infinite loop.
CVSS v3 base score: 7.5
Published: 2021-05-26T15:15Z
Repository: https://github.com/golang/net
Language: Go
Files changed: html/parse.go, html/parse_test.go
Commit date: 2021-05-20 17:08:46 +0000
Commit message: html: ignore templates nested within foreign content

Fixes #46288
Fixes CVE-2021-33194

Change-Id: I2fe39702de8e9aab29965c1526e377a6f9cdf056
Reviewed-on: https://go-review.googlesource.com/c/net/+/311090
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>

Diff: diff --git a/html/parse.go b/html/parse.go
index f91466f7..038941d7 100644
--- a/html/parse.go
+++ b/html/parse.go
@@ -661,10 +661,28 @@ func inHeadIM(p *parser) bool {
 			return true
 		case a.Head:
 			// Ignore the token.
 			return true
 		case a.Template:
+			// TODO: remove this divergence from the HTML5 spec.
+			//
+			// We don't handle all of the corner cases when mixing foreign
+			// content (i.e. <math> or <svg>) with <template>. Without this
+			// early return, we can get into an infinite loop, possibly because
+			// of the "TODO... further divergence" a little below.
+			//
+			// As a workaround, if we are mixing foreign content and templates,
+			// just ignore the rest of the HTML. Foreign content is rare and a
+			// relatively old HTML feature. Templates are also rare and a
+			// relatively new HTML feature. Their combination is very rare.
+			for _, e := range p.oe {
+				if e.Namespace != "" {
+					p.im = ignoreTheRemainingTokens
+					return true
+				}
+			}
+
 			p.addElement()
 			p.afe = append(p.afe, &scopeMarker)
 			p.framesetOK = false
 			p.im = inTemplateIM
 			p.templateStack = append(p.templateStack, inTemplateIM)
@@ -681,11 +699,11 @@ func inHeadIM(p *parser) bool {
 			return false
 		case a.Template:
 			if !p.oe.contains(a.Template) {
 				return true
 			}
-			// TODO: remove this divergence from the HTML5 spec.
+			// TODO: remove this further divergence from the HTML5 spec.
 			//
 			// See https://bugs.chromium.org/p/chromium/issues/detail?id=829668
 			p.generateImpliedEndTags()
 			for i := len(p.oe) - 1; i >= 0; i-- {
 				if n := p.oe[i]; n.Namespace == "" && n.DataAtom == a.Template {
@@ -2125,10 +2143,14 @@ func afterAfterFramesetIM(p *parser) bool {
 		// Ignore the token.
 	}
 	return true
 }
 
+func ignoreTheRemainingTokens(p *parser) bool {
+	return true
+}
+
 const whitespaceOrNUL = whitespace + "\x00"
 
 // Section 12.2.6.5
 func parseForeignContent(p *parser) bool {
 	switch p.tok.Type {
diff --git a/html/parse_test.go b/html/parse_test.go
index 58dce5fb..019333d5 100644
--- a/html/parse_test.go
+++ b/html/parse_test.go
@@ -265,10 +265,13 @@ func TestParser(t *testing.T) {
 					break
 				}
 				if err != nil {
 					t.Fatal(err)
 				}
+				if parseTestBlacklist[ta.text] {
+					continue
+				}
 
 				err = testParseCase(ta.text, ta.want, ta.context, ParseOptionEnableScripting(ta.scripting))
 
 				if err != nil {
 					t.Errorf("%s test #%d %q, %s", tf, i, ta.text, err)
@@ -377,10 +380,18 @@ func testParseCase(text, want, context string, opts ...ParseOption) (err error)
 	}
 
 	return nil
 }
 
+// Some test inputs are simply skipped - we would otherwise fail the test. We
+// blacklist such inputs from the parse test.
+var parseTestBlacklist = map[string]bool{
+	// See the a.Template TODO in inHeadIM.
+	`<math><template><mo><template>`:                                     true,
+	`<template><svg><foo><template><foreignObject><div></template><div>`: true,
+}
+
 // Some test input result in parse trees are not 'well-formed' despite
 // following the HTML5 recovery algorithms. Rendering and re-parsing such a
 // tree will not result in an exact clone of that tree. We blacklist such
 // inputs from the render test.
 var renderTestBlacklist = map[string]bool{
@@ -452,10 +463,21 @@ func TestNodeConsistency(t *testing.T) {
 func TestParseFragmentWithNilContext(t *testing.T) {
 	// This shouldn't panic.
 	ParseFragment(strings.NewReader("<p>hello</p>"), nil)
 }
 
+func TestParseFragmentForeignContentTemplates(t *testing.T) {
+	srcs := []string{
+		"<math><html><template><mn><template></template></template>",
+		"<math><math><head><mi><template>",
+	}
+	for _, src := range srcs {
+		// The next line shouldn't infinite-loop.
+		ParseFragment(strings.NewReader(src), nil)
+	}
+}
+
 func BenchmarkParser(b *testing.B) {
 	buf, err := ioutil.ReadFile("testdata/go1.html")
 	if err != nil {
 		b.Fatalf("could not read testdata/go1.html: %v", err)
 	}

Vulnerable code: 			// TODO: remove this divergence from the HTML5 spec.
Fixed code: 			// TODO: remove this divergence from the HTML5 spec.
			//
			// We don't handle all of the corner cases when mixing foreign
			// content (i.e. <math> or <svg>) with <template>. Without this
			// early return, we can get into an infinite loop, possibly because
			// of the "TODO... further divergence" a little below.
			//
			// As a workaround, if we are mixing foreign content and templates,
			// just ignore the rest of the HTML. Foreign content is rare and a
			// relatively old HTML feature. Templates are also rare and a
			// relatively new HTML feature. Their combination is very rare.
			for _, e := range p.oe {
				if e.Namespace != "" {
					p.im = ignoreTheRemainingTokens
					return true
				}
			}

			// TODO: remove this further divergence from the HTML5 spec.
func ignoreTheRemainingTokens(p *parser) bool {
	return true
}

				if parseTestBlacklist[ta.text] {
					continue
				}
// Some test inputs are simply skipped - we would otherwise fail the test. We
// blacklist such inputs from the parse test.
var parseTestBlacklist = map[string]bool{
	// See the a.Template TODO in inHeadIM.
	`<math><template><mo><template>`:                                     true,
	`<template><svg><foo><template><foreignObject><div></template><div>`: true,
}

func TestParseFragmentForeignContentTemplates(t *testing.T) {
	srcs := []string{
		"<math><html><template><mn><template></template></template>",
		"<math><math><head><mi><template>",
	}
	for _, src := range srcs {
		// The next line shouldn't infinite-loop.
		ParseFragment(strings.NewReader(src), nil)
	}
}