fix(go): add bounds checking to ByteVector #8776
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add missing bounds checking to ByteVector before slice operations in the Go FlatBuffers implementation. Relative offset and vector length are now checked against the buffer size. Instead of panicking, the code now returns nil. Regression test added. Signed-off-by: Ville Vesilehto <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Add missing bounds checking to
ByteVectorbefore slice operations in the Go FlatBuffers implementation. Relative offset and vector length are now checked against the buffer size. Instead of panicking, the code now returns nil. Regression test added.This change is very similar to #8684. The missing bounds check also affects the gRPC implementation, and acts as an unauthenticated DoS vector. The issue is reproducible with the Go gRPC greeter server.
Stacktrace below from a panic on a test environment with the following versions:
github.com/google/flatbuffers v25.9.23+incompatiblegoogle.golang.org/grpc v1.76.0panic: runtime error: slice bounds out of range [65543:28]goroutine 27 [running]:
github.com/google/flatbuffers/go.(*Table).ByteVector(0x102410f30?, 0x252e729?)
/git/flatbuffers/grpc/examples/go/greeter/server/vendor/github.com/google/flatbuffers/go/table.go:35 +0xe8
github.com/google/flatbuffers/grpc/examples/go/greeter/models.(*HelloRequest).Name(0x14000208020)
/git/flatbuffers/grpc/examples/go/greeter/server/vendor/github.com/google/flatbuffers/grpc/examples/go/greeter/models/HelloRequest.go:47 +0x40
main.(*greeterServer).SayHello(0x10297be20?, {0x102592300?, 0x1400009da38?}, 0x102400888?)
/git/flatbuffers/grpc/examples/go/greeter/server/main.go:23 +0x24
github.com/google/flatbuffers/grpc/examples/go/greeter/models._Greeter_SayHello_Handler({0x102592300, 0x1029bbb20}, {0x10261cf80, 0x1400020c000}, 0x1400007e2a0, 0x0)
/git/flatbuffers/grpc/examples/go/greeter/server/vendor/github.com/google/flatbuffers/grpc/examples/go/greeter/models/Greeter_grpc.go:109 +0x1b8
google.golang.org/grpc.(*Server).processUnaryRPC(0x1400013a1e0, {0x102620920, 0x140000aaea0}, 0x140000f4b40, 0x140001585a0, 0x10297be00, 0x0)
/git/flatbuffers/grpc/examples/go/greeter/server/vendor/google.golang.org/grpc/server.go:1335 +0xaf4
google.golang.org/grpc.(*Server).handleStream(0x1400013a1e0, {0x102620920, 0x140000aaea0}, 0x140000f4b40, 0x0)
/git/flatbuffers/grpc/examples/go/greeter/server/vendor/google.golang.org/grpc/server.go:1712 +0x720
google.golang.org/grpc.(*Server).serveStreams.func1.1()
/git/flatbuffers/grpc/examples/go/greeter/server/vendor/google.golang.org/grpc/server.go:947 +0xa0
created by google.golang.org/grpc.(*Server).serveStreams.func1 in goroutine 26
/git/flatbuffers/grpc/examples/go/greeter/server/vendor/google.golang.org/grpc/server.go:958 +0x12c
exit status 2
Running the regression test against
masteryields the following panic:--- FAIL: TestAll (0.00s) panic: runtime error: slice bounds out of range [100:8] [recovered, repanicked]goroutine 35 [running]:
testing.tRunner.func1.2({0x10106ad20, 0x140001f09f0})
/opt/homebrew/Cellar/go/1.25.4/libexec/src/testing/testing.go:1872 +0x190
testing.tRunner.func1()
/opt/homebrew/Cellar/go/1.25.4/libexec/src/testing/testing.go:1875 +0x31c
panic({0x10106ad20?, 0x140001f09f0?})
/opt/homebrew/Cellar/go/1.25.4/libexec/src/runtime/panic.go:783 +0x120
github.com/google/flatbuffers/go.(*Table).ByteVector(0x0?, 0x1000000?)
/git/flatbuffers/tests/go_gen/src/github.com/google/flatbuffers/go/table.go:35 +0x1ec
flatbuffers_test.CheckByteVectorBoundsChecking(0x140001e7f48)
/git/flatbuffers/tests/go_gen/src/flatbuffers_test/go_test.go:2588 +0x8c
flatbuffers_test.TestAll(0x1400018e700)
/git/flatbuffers/tests/go_gen/src/flatbuffers_test/go_test.go:136 +0x114
testing.tRunner(0x1400018e700, 0x10107e4d0)
/opt/homebrew/Cellar/go/1.25.4/libexec/src/testing/testing.go:1934 +0xc8
created by testing.(*T).Run in goroutine 1
/opt/homebrew/Cellar/go/1.25.4/libexec/src/testing/testing.go:1997 +0x364
exit status 2
FAIL flatbuffers_test 0.230s
Changes
Add bounds checking to
ByteVectorimplementation ingo/table.go. Invalid offset or out-of-bounds causes anilreturn. This is updated to function comment as well.The regression test includes three test cases, where two of them present malformed input. I've ran
tests/GoTest.shto make sure tests pass.Alternatives considered
I considered returning an error from
ByteVectorbut that would imply a breaking change.