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

encoding/json: Slice bounds out of range in json.Marshal #63379

Closed
AdamKorcz opened this issue Oct 4, 2023 · 7 comments
Closed

encoding/json: Slice bounds out of range in json.Marshal #63379

AdamKorcz opened this issue Oct 4, 2023 · 7 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.

Comments

@AdamKorcz
Copy link

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

go version go1.21.1 linux/amd64 and go version go1.21.0 linux/amd64

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

go env Output
$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/root/.cache/go-build'
GOENV='/root/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/root/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/root/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/root/.go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/root/.go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.1'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='clang'
CXX='clang++ -lresolv'
CGO_ENABLED='1'
GOMOD='/src/kyverno/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1734847614=/tmp/go-build -gno-record-gcc-switches'

Description

A fuzzer running on OSS-Fuzz ran into the following crash:

panic: runtime error: slice bounds out of range [23:22] [recovered]                                                                                                   [1/1988]
        panic: runtime error: slice bounds out of range [23:22] [recovered]
        panic: runtime error: slice bounds out of range [23:22]

goroutine 17 [running, locked to thread]:
main.catchPanics()
        ./main.3728049778.go:49 +0x27c
panic({0x460ca60?, 0x10c000e36eb8?})
        runtime/panic.go:914 +0x21f
encoding/json.(*encodeState).marshal.func1()
        encoding/json/encode.go:291 +0x113
panic({0x460ca60?, 0x10c000e36eb8?})
        runtime/panic.go:914 +0x21f
encoding/json.appendCompact({0x10c00101e729, 0x0, 0x8d7}, {0x10c000aa44e0, 0x19a, 0x19a}, 0x1)
        encoding/json/indent.go:71 +0xd2f
encoding/json.marshalerEncoder(0x10c001122340, {0x4700e60?, 0x10c000d2ae48?, 0x1?}, {0x30?, 0x85?})
        encoding/json/encode.go:446 +0x3c8
encoding/json.structEncoder.encode({{{0x10c000c2b200, 0x4, 0x4}, 0x10c000c68f60, 0x10c000c68f90}}, 0x10c001122340, {0x45a1b00?, 0x10c000d2ae30?, 0x10c000b96800?}, {0x0, ...})
        encoding/json/encode.go:706 +0x31e
encoding/json.structEncoder.encode({{{0x10c000c5c000, 0xb, 0x10}, 0x10c000d3f020, 0x10c000d3f050}}, 0x10c001122340, {0x4700cc0?, 0x10c000d2abe0?, 0x415a5e0?}, {0x0, ...})
        encoding/json/encode.go:706 +0x31e
encoding/json.arrayEncoder.encode({0x10c0011435c0?}, 0x10c001122340, {0x415a5e0?, 0x10c000a33b08?, 0x2?}, {0x23?, 0x0?})
        encoding/json/encode.go:849 +0x14f
encoding/json.sliceEncoder.encode({0x75d7c5?}, 0x10c001122340, {0x415a5e0?, 0x10c000a33b08?, 0x8?}, {0xc0?, 0x35?})
        encoding/json/encode.go:822 +0x639
encoding/json.structEncoder.encode({{{0x10c00141bb00, 0xd, 0x10}, 0x10c000d3f200, 0x10c000d3f230}}, 0x10c001122340, {0x4709f00?, 0x10c000a33b08?, 0x10c0011435c0?}, {0x0, ...}
)
        encoding/json/encode.go:706 +0x31e
encoding/json.structEncoder.encode({{{0x10c000a4c900, 0x5, 0x8}, 0x10c000d3f830, 0x10c000d3f860}}, 0x10c001122340, {0x45a1e00?, 0x10c000a33a00?, 0xc1?}, {0x0, ...})
        encoding/json/encode.go:706 +0x31e
encoding/json.ptrEncoder.encode({0x478e460?}, 0x10c001122340, {0x478e460?, 0x10c000a33a00?, 0x478e460?}, {0x0?, 0x3a?})
        encoding/json/encode.go:878 +0x4c5
encoding/json.(*encodeState).reflectValue(0x10c000e34cc0?, {0x478e460?, 0x10c000a33a00?, 0x455650?}, {0x0?, 0x0?})
        encoding/json/encode.go:323 +0x95
encoding/json.(*encodeState).marshal(0x10c000b1b2c0?, {0x478e460?, 0x10c000a33a00?}, {0x10?, 0xf8?})
        encoding/json/encode.go:295 +0x206
encoding/json.Marshal({0x478e460, 0x10c000a33a00})
        encoding/json/encode.go:162 +0xf7
github.com/kyverno/kyverno/pkg/validation/policy.hasVariables({0x4825fe8?, 0x10c000a33600?})
        github.com/kyverno/kyverno/pkg/validation/policy/validate.go:490 +0xc5
github.com/kyverno/kyverno/pkg/validation/policy.ValidateVariables({0x4825fe8, 0x10c000a33600}, 0x1)
        github.com/kyverno/kyverno/pkg/validation/policy/validate.go:386 +0x4c
github.com/kyverno/kyverno/pkg/validation/policy.Validate({0x4825fe8, 0x10c000a33600}, {0x0, 0x0}, {0x0, 0x0}, 0x1, {0x3ac31fc, 0x5})
        github.com/kyverno/kyverno/pkg/validation/policy/validate.go:129 +0x9b7
main.LibFuzzerFuzzValidatePolicy.FuzzValidatePolicy.func1(0x57c46d?, {0x61b000000784, 0x29d, 0x5fb})
        github.com/kyverno/kyverno/pkg/validation/policy/fuzz_test.go_fuzz.go:34 +0xfd
reflect.Value.call({0x4211a80?, 0x47ccde8?, 0x30?}, {0x3ac1eac, 0x4}, {0x10c000a4fb60, 0x2, 0x2?})
        reflect/value.go:596 +0x1fb1
reflect.Value.Call({0x4211a80?, 0x47ccde8?, 0x3accb2c?}, {0x10c000a4fb60, 0x2, 0x2})
        reflect/value.go:380 +0x18e
github.com/AdamKorcz/go-118-fuzz-build/testing.(*F).Fuzz(0x10c00009cd98, {0x4211a80?, 0x47ccde8?})
        github.com/AdamKorcz/go-118-fuzz-build@v0.0.0-20230306123547-8075edf89bb0/testing/f.go:177 +0x38d5
github.com/kyverno/kyverno/pkg/validation/policy.FuzzValidatePolicy(...)
        github.com/kyverno/kyverno/pkg/validation/policy/fuzz_test.go_fuzz.go:27
main.LibFuzzerFuzzValidatePolicy({0x61b000000780, 0x5ff, 0x5ff})
        ./main.3728049778.go:31 +0xed
main.LLVMFuzzerTestOneInput(0x0?, 0x5e1101?)
        ./main.3728049778.go:24 +0x6c

I am having some trouble reproducing it outside of OSS-Fuzz, however, it reproduces in OSS-Fuzz reliably. I hope the stack trace and other info provides enough context to locate the root cause.

@dsnet
Copy link
Member

dsnet commented Oct 4, 2023

A reproduction would be nice.

As far as I can locally reason about the code, start is provably <=len(src) except for on L72 where it could possibly the extended past len(src). This branch only occurs in the following states:

  • scanSkipSpace
  • scanEnd
  • scanError

scanError is uninteresting since we break out of the loop, so it's possible that we're incrementing start when we're not supposed to for scanSkipSpace or scanEnd. I didn't analyze the code beyond this point.

@dsnet
Copy link
Member

dsnet commented Oct 4, 2023

I am having some trouble reproducing it outside of OSS-Fuzz, however, it reproduces in OSS-Fuzz reliably.

I wonder if the bug (if one exists) is in newScanner and freeScanner, where the state machine is not properly reset.

@AlexanderYastrebov
Copy link
Contributor

I've walked it with debugger to create a reproducer https://go.dev/play/p/MLVjBpcNgbI

@dsnet
Copy link
Member

dsnet commented Oct 5, 2023

Excellent reproduction, thank you!

@AlexanderYastrebov
Copy link
Contributor

I'll create a CL

@AdamKorcz
Copy link
Author

I've walked it with debugger to create a reproducer https://go.dev/play/p/MLVjBpcNgbI

Amazing! Thank you very much!

AlexanderYastrebov added a commit to AlexanderYastrebov/go that referenced this issue Oct 5, 2023
stateEndTop did not return scanError which caused a panic inside of
appendCompact if top-level value was followed by an escaped character.

It was also a single place where return value of *scanner.error
was not used.

Fixes golang#63379
@gopherbot
Copy link

Change https://go.dev/cl/533275 mentions this issue: encoding/json: handle escaped content after the top-level value

AlexanderYastrebov added a commit to AlexanderYastrebov/go that referenced this issue Oct 5, 2023
CL 469555 changed Compact to use append instead of bytes.Buffer.

appendCompact iterates over input src slice and performs escaping
of certain characters.
To optimize copying it does not copy characters one by one
but keeps track of the start offset of the data to copy when
it reaches next character to escape or the end of the input.

This start offset may become greater than input character offset
so copying of preceding data should check this condition.

CL 469555 removed boundary checks for copying data preceding
escaped characters and this change restores them.

Fixes golang#63379
@prattmic prattmic added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants