kisielk / og-rek

ogórek is a Go library for encoding and decoding pickles.
MIT License
60 stars 16 forks source link

decoder: Tuple cannot be used as dict key #50

Open navytux opened 6 years ago

navytux commented 6 years ago

If tuple is used directly, for example:

In [11]: s = "((tNd."

In [19]: dis(s)
    0: (    MARK
    1: (        MARK
    2: t            TUPLE      (MARK at 1)
    3: N        NONE
    4: d        DICT       (MARK at 0)
    5: .    STOP
highest protocol among opcodes = 0

In [20]: loads(s)
Out[20]: {(): None}

our Decoder gives:

pickle: loadDict: invalid key type ogórek.Tuple

However the pickle is valid by Python rules because there tuple is immutable (and thus hashable).

However the check inside Decoder is not complete - for example if we hook Tuple into another type whos is comparable (e.g. Ref here):

In [11]: s = "((tQNd."

In [15]: dis(s)
    0: (    MARK
    1: (        MARK
    2: t            TUPLE      (MARK at 1)
    3: Q        BINPERSID
    4: N        NONE
    5: d        DICT       (MARK at 0)
    6: .    STOP
highest protocol among opcodes = 1

our Decoder crashes:

panic: runtime error: hash of unhashable type ogórek.Tuple

goroutine 1 [running]:
github.com/kisielk/og-rek.(*Decoder).loadDict(0xc00009efd0, 0xc00000a264, 0x0)
        /tmp/go-fuzz-build914098789/gopath/src/github.com/kisielk/og-rek/ogorek.go:819 +0x32b
github.com/kisielk/og-rek.(*Decoder).Decode(0xc00009efd0, 0xc00009aa10, 0xc00009efd0, 0x464f39, 0x4)
        /tmp/go-fuzz-build914098789/gopath/src/github.com/kisielk/og-rek/ogorek.go:242 +0x14fa
github.com/kisielk/og-rek.Fuzz(0x7f62339cc000, 0x6, 0x200000, 0x3)
        /tmp/go-fuzz-build914098789/gopath/src/github.com/kisielk/og-rek/fuzz.go:15 +0xdf
go-fuzz-dep.Main(0x524df8)
        /tmp/go-fuzz-build914098789/goroot/src/go-fuzz-dep/main.go:49 +0xad
main.main()
        /tmp/go-fuzz-build914098789/gopath/src/github.com/kisielk/og-rek/go.fuzz.main/main.go:10 +0x2d
exit status 2
navytux commented 6 years ago

About the second crash -- see https://github.com/kisielk/og-rek/issues/30#issuecomment-423803200 -- it seems likely we will have to rework our "value can be used as dict key" logic from reflect.Type.Comparable into using panic/recover.

navytux commented 1 month ago

I think to the possible extent this issue is fixed by PyDict mode: https://github.com/kisielk/og-rek/commit/3bf6c92de63baa92d07a9e85edab0ae017fd9eb7 (https://github.com/kisielk/og-rek/pull/75).