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

GODRIVER-1923 Error if BSON cstrings contain null bytes #622

Merged
merged 5 commits into from Mar 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion bson/bsonrw/value_writer.go
Expand Up @@ -12,6 +12,7 @@ import (
"io"
"math"
"strconv"
"strings"
"sync"

"go.mongodb.org/mongo-driver/bson/bsontype"
Expand Down Expand Up @@ -247,7 +248,12 @@ func (vw *valueWriter) invalidTransitionError(destination mode, name string, mod
func (vw *valueWriter) writeElementHeader(t bsontype.Type, destination mode, callerName string, addmodes ...mode) error {
switch vw.stack[vw.frame].mode {
case mElement:
vw.buf = bsoncore.AppendHeader(vw.buf, t, vw.stack[vw.frame].key)
key := vw.stack[vw.frame].key
if !isValidCString(key) {
return errors.New("BSON element key cannot contain null bytes")
}
divjotarora marked this conversation as resolved.
Show resolved Hide resolved

vw.buf = bsoncore.AppendHeader(vw.buf, t, key)
case mValue:
// TODO: Do this with a cache of the first 1000 or so array keys.
vw.buf = bsoncore.AppendHeader(vw.buf, t, strconv.Itoa(vw.stack[vw.frame].arrkey))
Expand Down Expand Up @@ -430,6 +436,9 @@ func (vw *valueWriter) WriteObjectID(oid primitive.ObjectID) error {
}

func (vw *valueWriter) WriteRegex(pattern string, options string) error {
if !isValidCString(pattern) || !isValidCString(options) {
iwysiu marked this conversation as resolved.
Show resolved Hide resolved
return errors.New("BSON regex values cannot contain null bytes")
}
if err := vw.writeElementHeader(bsontype.Regex, mode(0), "WriteRegex"); err != nil {
return err
}
Expand Down Expand Up @@ -602,3 +611,7 @@ func (vw *valueWriter) writeLength() error {
vw.buf[start+3] = byte(length >> 24)
return nil
}

func isValidCString(cs string) bool {
return !strings.ContainsRune(cs, '\x00')
}
33 changes: 33 additions & 0 deletions bson/marshal_test.go
Expand Up @@ -8,6 +8,7 @@ package bson

import (
"bytes"
"errors"
"fmt"
"reflect"
"testing"
Expand Down Expand Up @@ -267,3 +268,35 @@ func TestCachingEncodersNotSharedAcrossRegistries(t *testing.T) {
})
})
}

func TestNullBytes(t *testing.T) {
t.Run("element keys", func(t *testing.T) {
doc := D{{"a\x00", "foobar"}}
kevinAlbs marked this conversation as resolved.
Show resolved Hide resolved
res, err := Marshal(doc)
want := errors.New("BSON element key cannot contain null bytes")
assert.Equal(t, want, err, "expected Marshal error %v, got error %v with result %q", want, err, Raw(res))
})

t.Run("regex values", func(t *testing.T) {
wantErr := errors.New("BSON regex values cannot contain null bytes")

testCases := []struct {
name string
pattern string
options string
}{
{"null bytes in pattern", "a\x00", "i"},
{"null bytes in options", "pattern", "i\x00"},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
regex := primitive.Regex{
Pattern: tc.pattern,
Options: tc.options,
}
res, err := Marshal(D{{"foo", regex}})
assert.Equal(t, wantErr, err, "expected Marshal error %v, got error %v with result %q", wantErr, err, Raw(res))
})
}
})
}
26 changes: 21 additions & 5 deletions x/bsonx/bsoncore/bsoncore.go
Expand Up @@ -30,17 +30,21 @@ import (
"fmt"
"math"
"strconv"
"strings"
"time"

"go.mongodb.org/mongo-driver/bson/bsontype"
"go.mongodb.org/mongo-driver/bson/primitive"
)

// EmptyDocumentLength is the length of a document that has been started/ended but has no elements.
const EmptyDocumentLength = 5

// nullTerminator is a string version of the 0 byte that is appended at the end of cstrings.
const nullTerminator = string(byte(0))
const (
// EmptyDocumentLength is the length of a document that has been started/ended but has no elements.
EmptyDocumentLength = 5
// nullTerminator is a string version of the 0 byte that is appended at the end of cstrings.
nullTerminator = string(byte(0))
invalidKeyPanicMsg = "BSON element keys cannot contain null bytes"
invalidRegexPanicMsg = "BSON regex values cannot contain null bytes"
)

// AppendType will append t to dst and return the extended buffer.
func AppendType(dst []byte, t bsontype.Type) []byte { return append(dst, byte(t)) }
Expand All @@ -51,6 +55,10 @@ func AppendKey(dst []byte, key string) []byte { return append(dst, key+nullTermi
// AppendHeader will append Type t and key to dst and return the extended
// buffer.
func AppendHeader(dst []byte, t bsontype.Type, key string) []byte {
if !isValidCString(key) {
panic(invalidKeyPanicMsg)
}

dst = AppendType(dst, t)
dst = append(dst, key...)
return append(dst, 0x00)
Expand Down Expand Up @@ -430,6 +438,10 @@ func AppendNullElement(dst []byte, key string) []byte { return AppendHeader(dst,

// AppendRegex will append pattern and options to dst and return the extended buffer.
func AppendRegex(dst []byte, pattern, options string) []byte {
if !isValidCString(pattern) || !isValidCString(options) {
panic(invalidRegexPanicMsg)
}

return append(dst, pattern+nullTerminator+options+nullTerminator...)
}

Expand Down Expand Up @@ -844,3 +856,7 @@ func appendBinarySubtype2(dst []byte, subtype byte, b []byte) []byte {
dst = appendLength(dst, int32(len(b)))
return append(dst, b...)
}

func isValidCString(cs string) bool {
return !strings.ContainsRune(cs, '\x00')
}
46 changes: 46 additions & 0 deletions x/bsonx/bsoncore/bsoncore_test.go
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/google/go-cmp/cmp"
"go.mongodb.org/mongo-driver/bson/bsontype"
"go.mongodb.org/mongo-driver/bson/primitive"
"go.mongodb.org/mongo-driver/internal/testutil/assert"
)

func noerr(t *testing.T, err error) {
Expand Down Expand Up @@ -899,6 +900,51 @@ func TestBuild(t *testing.T) {
}
}

func TestNullBytes(t *testing.T) {
// Helper function to execute the provided callback and assert that it panics with the expected message. The
// createBSONFn callback should create a BSON document/array/value and return the stringified version.
assertBSONCreationPanics := func(t *testing.T, createBSONFn func(), expected string) {
t.Helper()

defer func() {
got := recover()
divjotarora marked this conversation as resolved.
Show resolved Hide resolved
assert.Equal(t, expected, got, "expected panic with error %v, got error %v", expected, got)
}()
createBSONFn()
}

t.Run("element keys", func(t *testing.T) {
createDocFn := func() {
NewDocumentBuilder().AppendString("a\x00", "foo")
}
assertBSONCreationPanics(t, createDocFn, invalidKeyPanicMsg)
})
t.Run("regex values", func(t *testing.T) {
testCases := []struct {
name string
pattern string
options string
}{
{"null bytes in pattern", "a\x00", "i"},
{"null bytes in options", "pattern", "i\x00"},
}
for _, tc := range testCases {
t.Run(tc.name+"-AppendRegexElement", func(t *testing.T) {
createDocFn := func() {
AppendRegexElement(nil, "foo", tc.pattern, tc.options)
}
assertBSONCreationPanics(t, createDocFn, invalidRegexPanicMsg)
})
t.Run(tc.name+"-AppendRegex", func(t *testing.T) {
createValFn := func() {
AppendRegex(nil, tc.pattern, tc.options)
}
assertBSONCreationPanics(t, createValFn, invalidRegexPanicMsg)
})
}
})
}

func compareDecimal128(d1, d2 primitive.Decimal128) bool {
d1H, d1L := d1.GetBytes()
d2H, d2L := d2.GetBytes()
Expand Down