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

Invalid simple_keys now cause panics later in decode #548

Closed
cjcullen opened this issue Nov 19, 2019 · 1 comment · Fixed by kubernetes-sigs/yaml#29
Closed

Invalid simple_keys now cause panics later in decode #548

cjcullen opened this issue Nov 19, 2019 · 1 comment · Fixed by kubernetes-sigs/yaml#29

Comments

@cjcullen
Copy link

@cjcullen cjcullen commented Nov 19, 2019

Looks like a test case that I missed in #543.

The following test in unmarshalErrorTests in decode_test.go passes before that PR, and fails after.

	{"a:\n  1:\nb\n  2:", ".*could not find expected ':'"},

I get a panic:

$ go test
...
... Panic: attempted to parse unknown event: none (PC=0x42FD21)

/usr/lib/google-golang/src/runtime/panic.go:679
  in gopanic
yaml.go:249
  in handleErr
/usr/lib/google-golang/src/runtime/panic.go:679
  in gopanic
decode.go:158
  in parser.parse
decode.go:217
  in parser.mapping
decode.go:149
  in parser.parse
decode.go:217
  in parser.mapping
decode.go:149
  in parser.parse
decode.go:175
  in parser.document
decode.go:153
  in parser.parse
yaml.go:142
  in unmarshal
yaml.go:81
  in Unmarshal
decode_test.go:889
  in S.TestUnmarshalErrors
/usr/lib/google-golang/src/reflect/value.go:321
  in Value.Call
/usr/lib/google-golang/src/runtime/asm_amd64.s:1357
  in goexit
OOPS: 30 passed, 2 PANICKED
--- FAIL: Test (2.83s)
FAIL
exit status 1
FAIL	gopkg.in/yaml.v2	2.885s

I think we are no longer catching the "stale" case now that we only process the simple_keys stack from the top.

@niemeyer
Copy link
Contributor

@niemeyer niemeyer commented Nov 19, 2019

No, it's a different issue. Sorry for the bug. This is fixed in 1f64d61.

niemeyer added a commit that referenced this issue Nov 19, 2019
thaJeztah added a commit to thaJeztah/yaml that referenced this issue Nov 28, 2019
full diff: go-yaml/yaml@v2.2.2...v2.2.7

includes:

- go-yaml/yaml@caeefd8
  addresses CVE-2019-11253 JSON/YAML parsing vulnerable to resource exhaustion attack
- go-yaml/yaml#171 Tighten restrictions on float decoding
- go-yaml/yaml#515 Add large document benchmarks, tune alias heuristic, add max depth limits
- go-yaml/yaml@f90ceb4
  fixes go-yaml/yaml#529 yaml.Unmarshal crashes on "assignment to entry in nil map"
- go-yaml/yaml#543 Port stale simple_keys fix to v2
- go-yaml/yaml@1f64d61
  fixes go-yaml/yaml#548 Invalid simple_keys now cause panics later in decode

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/yaml that referenced this issue Nov 28, 2019
full diff: go-yaml/yaml@v2.2.2...v2.2.7

includes:

- go-yaml/yaml@caeefd8
  addresses CVE-2019-11253 JSON/YAML parsing vulnerable to resource exhaustion attack
- go-yaml/yaml#171 Tighten restrictions on float decoding
- go-yaml/yaml#515 Add large document benchmarks, tune alias heuristic, add max depth limits
- go-yaml/yaml@f90ceb4
  fixes go-yaml/yaml#529 yaml.Unmarshal crashes on "assignment to entry in nil map"
- go-yaml/yaml#543 Port stale simple_keys fix to v2
- go-yaml/yaml@1f64d61
  fixes go-yaml/yaml#548 Invalid simple_keys now cause panics later in decode

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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 a pull request may close this issue.

2 participants