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/gob: panic on random input #9649

Closed
ThomasHabets opened this issue Jan 21, 2015 · 8 comments
Closed

encoding/gob: panic on random input #9649

ThomasHabets opened this issue Jan 21, 2015 · 8 comments
Milestone

Comments

@ThomasHabets
Copy link
Contributor

100% reproducible panic inside the gob decoder. Version is the binary package from golang.org. Panic on user data seems like the wrong thing to do, even if it could possibly be because Decode was called after Decode returned error (I don't know that this is always the case).

I encountered this on a real but corrupt gob file, but it seems that just generating 10MB of garbage does it too.

$ uname -a
Linux xxxxxxxx 3.16.0-0.bpo.4-amd64 #1 SMP Debian 3.16.7-ckt2-1~bpo70+1 (2014-12-08) x86_64 GNU/Linux
$ go version
go version go1.4.1 linux/amd64
$ dd if=/dev/urandom bs=1M count=10 of=crash.data
$ go build crash.go && ./crash
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x18 pc=0x44b648]

goroutine 1 [running]:
encoding/gob.catchError(0xc20807a070)
        /usr/local/go/src/encoding/gob/error.go:38 +0x96
encoding/gob.(*Decoder).decodeValue(0xc20807a000, 0xc200000000, 0x691240, 0xc2080837a0, 0xd9)
        /usr/local/go/src/encoding/gob/decode.go:1159 +0x258
encoding/gob.(*Decoder).DecodeValue(0xc20807a000, 0x600da0, 0xc2080837a0, 0x16, 0x0, 0x0)
        /usr/local/go/src/encoding/gob/decoder.go:210 +0x1d9
encoding/gob.(*Decoder).Decode(0xc20807a000, 0x600da0, 0xc2080837a0, 0x0, 0x0)
        /usr/local/go/src/encoding/gob/decoder.go:185 +0x297
main.main()
        /xxx/var/log/merged/tmp/crash.go:54 +0x192
$ cat crash.go
package main

import (
        "encoding/gob"
        "log"
        "os"
)

type foo struct{} // for actual code this struct is of course not empty, but crash is reproducible with it empty.

func main() {
        f, err := os.Open("crash.data")
        if err != nil {
                log.Fatal(err)
        }
        dec := gob.NewDecoder(f)
        for {   
                var i foo // i being 'int' does not trigger it.
                dec.Decode(&i) // I get one or more error returns before it crashes.
        }
}
@bradfitz bradfitz added this to the Go1.5 milestone Jan 21, 2015
@bradfitz bradfitz assigned bradfitz and robpike and unassigned bradfitz Jan 21, 2015
@bradfitz
Copy link
Contributor

It's not supposed to panic. That's why it returns an error.

@mattn
Copy link
Member

mattn commented Jan 21, 2015

I'm on windows but can't reproduce. I'm already waiting for 3min.

@bradfitz
Copy link
Contributor

The stack suggests the problem:

   dec.wireType[wireId] != nil && len(dec.wireType[wireId].StructT.Field) > 0 {

The StructT is nil.

While somebody is there, I would avoid the double map lookup too. (which would help even if #5147 is fixed).

@osocurioso
Copy link
Contributor

I'm able to reproduce on Linux amd64. Would you mind a CL for review?

@bradfitz
Copy link
Contributor

@osocurioso, send away.

@osocurioso
Copy link
Contributor

@kortschak
Copy link
Contributor

Taking a closer look at this. The issue seems to be that the error examined at https://github.com/golang/go/blob/master/src/encoding/gob/decode.go#L1111 is never non-nil; the error return from compileDec is not used as far as I can see - all errors are sent via the error_ panic function and caught by catchError at https://github.com/golang/go/blob/master/src/encoding/gob/decode.go#L1145 missing this check.

This means that the decoderMap entry is never deleted and so the decoder then sees the nil value.

@osocurioso
Copy link
Contributor

@kortschak: You are right. I've update the CL to fix the real issue.

@robpike robpike closed this as completed in 8e6cf5f Apr 8, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
@rsc rsc unassigned robpike Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants