x/net/html: html.Parse() panic caused by malformed data

[tr3ee]

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.10.3 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GORACE=""
GOTMPDIR=""
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

package main

import (
	"strings"

	"golang.org/x/net/html"
)

func main() {
	r := strings.NewReader("<math><template><mo><template>")
	html.Parse(r)
}

What did you expect to see?

No panic exit

What did you see instead?

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x486c7f]

goroutine 1 [running]:
golang.org/x/net/html.(*parser).parseCurrentToken(0xc42011e410)
        /path/to/gopath/src/golang.org/x/net/html/parse.go:2180 +0x7f
golang.org/x/net/html.(*parser).parse(0xc42011e410, 0xc420160000, 0xc42011e340)
        /path/to/gopath/src/golang.org/x/net/html/parse.go:2206 +0x5b
golang.org/x/net/html.Parse(0x4d5b00, 0xc42011c0a0, 0xc420114150, 0xc4200dff78, 0xc420110058)
        /path/to/gopath/src/golang.org/x/net/html/parse.go:2232 +0xdb
main.main()
        /path/to/gopath/src/html/main.go:11 +0x73
exit status 2

[odeke-em]

Thank you @tr3ee for reporting this issue!

/cc @nigeltao @namusyaka

[tr3ee]

CVE-2018-17142 has been assigned to this.

[nigeltao]

@namusyaka (already @-mentioned) wrote the <template> parsing code, and is probably the best person to fix this.

@bradfitz may know if we need to do anything with the CVE identifier.

[bradfitz]

The CVE existing in a comment in this bug is sufficient (for searchability).

[thatnerdjosh]

Not sure if this helps, as I don't know much about the HTML parsing spec, but I was looking into this being a new contributor and trying to learn the code base... it seems like the Insertion Mode struct attribute for parser is nil, causing a nil pointer dereference error.

I don't mind diving into it further to see about finding the root cause and fix it if @namusyaka wouldn't mind and isn't already working on it.

EDIT: As an alternative and/or temporary fix... @tr3ee what are your thoughts on using defer, panic, recover

https://blog.golang.org/defer-panic-and-recover

package main

import (
  "fmt"
  "strings"

  "golang.org/x/net/html"
)

func rescue() {
  r := recover()

  if r != nil {
    // Necessary logic to follow to recover from panic
  }
}

func main() {
  defer rescue()
  r := strings.NewReader("<math><template><mo><template>")
  html.Parse(r)
}

[tr3ee]

hello @NerdsvilleCEO ,
this type of fix is not recommended and can only be used as a temporary mitigation effect, which can have an impact on performance.

[thatnerdjosh]

@nigeltao what do you think about doing a nil pointer check and returning an error from parseCurrentToken if p.im is nil? I know it doesn't fix it at the root of the issue but it's a start, are there any disadvantages to doing this?

[nigeltao]

The proper fix is to not set p.im to nil in the first place.

[namusyaka]

I can take a look at this tomorrow or in a few days

[gopherbot]

Change https://golang.org/cl/136875 mentions this issue: html: don't set im if <template> doesn't have HTML namespace