Commit 5dbc8a1b authored by Kirill Smelkov's avatar Kirill Smelkov Committed by Kamil Kisiel

decoder: Don't allow mark to be returned as pickle result

A pickle is considered as invalid if it tries to return MARK as the
result by both Python2 and Python3, e.g.:

        In [2]: pickle.loads(b"(.")
        ---------------------------------------------------------------------------
        UnpicklingError                           Traceback (most recent call last)
        <ipython-input-2-0c142c82b126> in <module>()
        ----> 1 pickle.loads(b"(.")

        UnpicklingError: unexpected MARK found

However until now, despite mark is unexported ogórek type, we were
allowing for it to be returned just ok.

The problem was caught by decode/encode roundtrip fuzz tests, e.g.

	"(Q."

panic: protocol 1: decode·encode != identity:
have: ogórek.Ref{Pid:map[interface {}]interface {}{}}
want: ogórek.Ref{Pid:ogórek.mark{}}

goroutine 1 [running]:
github.com/kisielk/og-rek.Fuzz(0x7fbe6c15f000, 0x3, 0x200000, 0x3)
        /tmp/go-fuzz-build697921479/gopath/src/github.com/kisielk/og-rek/fuzz.go:87 +0x604
go-fuzz-dep.Main(0x524d78)
        /tmp/go-fuzz-build697921479/goroot/src/go-fuzz-dep/main.go:49 +0xad
main.main()
        /tmp/go-fuzz-build697921479/gopath/src/github.com/kisielk/og-rek/go.fuzz.main/main.go:10 +0x2d
exit status 2
parent 302c79ea
......@@ -105,6 +105,7 @@ const (
var errNotImplemented = errors.New("unimplemented opcode")
var ErrInvalidPickleVersion = errors.New("invalid pickle version")
var errNoMarker = errors.New("no marker in stack")
var errNoMarkReturn = errors.New("pickle: MARK cannot be returned")
var errStackUnderflow = errors.New("pickle: stack underflow")
// OpcodeError is the error that Decode returns when it sees unknown pickle opcode.
......@@ -320,7 +321,18 @@ loop:
return nil, err
}
}
return d.pop()
ret, err := d.pop()
// don't allow mark to be returned
switch ret.(type) {
case mark:
ret, err = nil, errNoMarkReturn
default: // ok
}
return ret, err
}
// readLine reads next line from pickle stream
......
......@@ -561,6 +561,9 @@ func TestDecodeError(t *testing.T) {
// BINSTRING with big len and no data
// (might cause out-of-memory DOS if buffer is preallocated blindly)
"T\xff\xff\xff\xff.",
// it is invalid to return mark object
"(.",
}
for _, tt := range testv {
buf := bytes.NewBufferString(tt)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment