[release-branch.go1.21] encoding/gob: prevent panic from index out of range in Decode...

I believe this bug is introduced by CL 460543 which optimizes the allocations
by changing the type of `idToType` from map to slice, but didn't update the
access code in `Decoder.typeString` that is safe for map but not for slice.

For #62117
Fixes #62154

Change-Id: I0f2e4cc2f34c54dada1f83458ba512a6fde6dcbe
Reviewed-on: https://go-review.googlesource.com/c/go/+/520757
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Andy Pan <panjf2000@gmail.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
(cherry picked from commit ba626ac327f45a6d9d211fddd5b48e321fa0702a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/521156
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>

diff --git a/src/encoding/gob/codec_test.go b/src/encoding/gob/codec_test.go
index 28cd6088af16ff076702d3ecc4f1959131c892ed..1b8f195986fa2f2fb2cb8f54201036fbff12fcaf 100644 (file)
--- a/src/encoding/gob/codec_test.go
+++ b/src/encoding/gob/codec_test.go
@@ -1607,3 +1607,15 @@ func TestLargeSlice(t *testing.T) {
                testEncodeDecode(t, st, rt)
        })
 }
+
+func TestLocalRemoteTypesMismatch(t *testing.T) {
+       // Test data is from https://go.dev/issue/62117.
+       testData := []byte{9, 127, 3, 1, 2, 255, 128, 0, 0, 0, 3, 255, 128, 0}
+
+       var v []*struct{}
+       buf := bytes.NewBuffer(testData)
+       err := NewDecoder(buf).Decode(&v)
+       if err == nil {
+               t.Error("Encode/Decode: expected error but got err == nil")
+       }
+}

diff --git a/src/encoding/gob/decode.go b/src/encoding/gob/decode.go
index 76ea332e5dee51205c8e1e5d0b44deecf4f50c9a..c0b054ef8022b1b19e59cd0810cbdc1f5bffde59 100644 (file)
--- a/src/encoding/gob/decode.go
+++ b/src/encoding/gob/decode.go
@@ -1082,7 +1082,7 @@ func (dec *Decoder) compatibleType(fr reflect.Type, fw typeId, inProgress map[re
 func (dec *Decoder) typeString(remoteId typeId) string {
        typeLock.Lock()
        defer typeLock.Unlock()
-       if t := idToType[remoteId]; t != nil {
+       if t := idToType(remoteId); t != nil {
                // globally known type.
                return t.string()
        }

diff --git a/src/encoding/gob/type.go b/src/encoding/gob/type.go
index 205a0b369450c9d964752cbc688ac6cf94d2b694..bd7d91994c79a847343360b5e2010fbca85733ff 100644 (file)
--- a/src/encoding/gob/type.go
+++ b/src/encoding/gob/type.go
@@ -173,9 +173,18 @@ type gobType interface {
        safeString(seen map[typeId]bool) string
 }
 
-var types = make(map[reflect.Type]gobType, 32)
-var idToType = make([]gobType, 1, firstUserId)
-var builtinIdToTypeSlice [firstUserId]gobType // set in init() after builtins are established
+var (
+       types                = make(map[reflect.Type]gobType, 32)
+       idToTypeSlice        = make([]gobType, 1, firstUserId)
+       builtinIdToTypeSlice [firstUserId]gobType // set in init() after builtins are established
+)
+
+func idToType(id typeId) gobType {
+       if id < 0 || int(id) >= len(idToTypeSlice) {
+               return nil
+       }
+       return idToTypeSlice[id]
+}
 
 func builtinIdToType(id typeId) gobType {
        if id < 0 || int(id) >= len(builtinIdToTypeSlice) {
@@ -189,16 +198,16 @@ func setTypeId(typ gobType) {
        if typ.id() != 0 {
                return
        }
-       nextId := typeId(len(idToType))
+       nextId := typeId(len(idToTypeSlice))
        typ.setId(nextId)
-       idToType = append(idToType, typ)
+       idToTypeSlice = append(idToTypeSlice, typ)
 }
 
 func (t typeId) gobType() gobType {
        if t == 0 {
                return nil
        }
-       return idToType[t]
+       return idToType(t)
 }
 
 // string returns the string representation of the type associated with the typeId.
@@ -277,14 +286,14 @@ func init() {
        checkId(21, mustGetTypeInfo(reflect.TypeOf((*fieldType)(nil)).Elem()).id)
        checkId(23, mustGetTypeInfo(reflect.TypeOf((*mapType)(nil)).Elem()).id)
 
-       copy(builtinIdToTypeSlice[:], idToType)
+       copy(builtinIdToTypeSlice[:], idToTypeSlice)
 
        // Move the id space upwards to allow for growth in the predefined world
        // without breaking existing files.
-       if nextId := len(idToType); nextId > firstUserId {
+       if nextId := len(idToTypeSlice); nextId > firstUserId {
                panic(fmt.Sprintln("nextId too large:", nextId))
        }
-       idToType = idToType[:firstUserId]
+       idToTypeSlice = idToTypeSlice[:firstUserId]
        registerBasics()
        wireTypeUserInfo = userType(wireTypeType)
 }
@@ -526,7 +535,7 @@ func newTypeObject(name string, ut *userTypeInfo, rt reflect.Type) (gobType, err
        case reflect.Struct:
                st := newStructType(name)
                types[rt] = st
-               idToType[st.id()] = st
+               idToTypeSlice[st.id()] = st
                for i := 0; i < t.NumField(); i++ {
                        f := t.Field(i)
                        if !isSent(&f) {