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
Assignees
Milestone

Comments

@ThomasHabets
Copy link
Contributor

@ThomasHabets ThomasHabets commented Jan 21, 2015

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

@bradfitz bradfitz commented Jan 21, 2015

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

@mattn
Copy link
Member

@mattn mattn commented Jan 21, 2015

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

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jan 21, 2015

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

@osocurioso osocurioso commented Jan 28, 2015

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

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jan 29, 2015

@osocurioso, send away.

@osocurioso
Copy link
Contributor

@osocurioso osocurioso commented Jan 29, 2015

@kortschak
Copy link
Contributor

@kortschak kortschak commented Apr 7, 2015

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

@osocurioso osocurioso commented Apr 7, 2015

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

@robpike robpike closed this in 8e6cf5f Apr 8, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.