Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scanner: Update prevPos even when returning utf8.RuneError. #240

Merged
merged 2 commits into from Mar 20, 2018

Conversation

octo
Copy link
Contributor

@octo octo commented Mar 19, 2018

This fixes an off-by-one error that's triggered by a combination of next() returning utf8.RuneError and unread() being called.

Without this fix, the provided unit test crashes, the the following stack trace. This is because the off-by-one error triggers the here-doc marker, which is a multi-byte unicode codepoint, to only be partially included in the regular expression, causing a regexp compile error.

panic: regexp: Compile("[[:space:]]*<\xc8\\z"): error parsing regexp: invalid UTF-8: `�\z`

goroutine 32 [running]:
testing.tRunner.func1(0xc4200cae10)
        /usr/lib/google-golang/src/testing/testing.go:742 +0x29d
panic(0x507a00, 0xc420290690)
        /usr/lib/google-golang/src/runtime/panic.go:505 +0x229
regexp.MustCompile(0xc420289e10, 0x10, 0xc420087680)
        /usr/lib/google-golang/src/regexp/regexp.go:240 +0x171
github.com/hashicorp/hcl/hcl/scanner.(*Scanner).scanHeredoc(0xc4200878c0)
        gopath/src/github.com/hashicorp/hcl/hcl/scanner/scanner.go:444 +0x3a9
github.com/hashicorp/hcl/hcl/scanner.(*Scanner).Scan(0xc4200878c0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
        gopath/src/github.com/hashicorp/hcl/hcl/scanner/scanner.go:186 +0x3e5

Notice the off-by-one error in the regexp: the regular expression is not supposed to contain the < character.

Kudos to dvyukov/go-fuzz for finding this!

```
panic: regexp: Compile("[[:space:]]*<\xc8\\z"): error parsing regexp: invalid UTF-8: `�\z`

goroutine 32 [running]:
testing.tRunner.func1(0xc4200cae10)
        /usr/lib/google-golang/src/testing/testing.go:742 +0x29d
panic(0x507a00, 0xc420290690)
        /usr/lib/google-golang/src/runtime/panic.go:505 +0x229
regexp.MustCompile(0xc420289e10, 0x10, 0xc420087680)
        /usr/lib/google-golang/src/regexp/regexp.go:240 +0x171
github.com/hashicorp/hcl/hcl/scanner.(*Scanner).scanHeredoc(0xc4200878c0)
        gopath/src/github.com/hashicorp/hcl/hcl/scanner/scanner.go:444 +0x3a9
github.com/hashicorp/hcl/hcl/scanner.(*Scanner).Scan(0xc4200878c0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
        gopath/src/github.com/hashicorp/hcl/hcl/scanner/scanner.go:186 +0x3e5
```
The calling code will still call unread(), causing panics.
This fixes the TestScanHeredocRegexpCompile() unit test.
@mitchellh
Copy link
Contributor

Great find, thank you!

@mitchellh mitchellh merged commit f40e974 into hashicorp:master Mar 20, 2018
@octo octo deleted the scanner-next branch March 22, 2018 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants