go-json-experiment / json

Experimental implementation of a proposed v2 encoding/json package
BSD 3-Clause "New" or "Revised" License
341 stars 11 forks source link

Stackoverflows while decoding recursive pointers #12

Open sugawarayuuta opened 9 months ago

sugawarayuuta commented 9 months ago

Speaking of correctness, I found an issue regarding recursive structure in various json libraries. consider this example:

type (
    recursivePointer *recursivePointer
    recursiveMap     map[string]recursiveMap
    recursiveSlice   []recursiveSlice
)

func main() {
    json.Unmarshal([]byte("123"), new(recursivePointer))
}
O = resulted in an error, X = stack-overflow, ? = hanging library enc-pointer enc-slice enc-map dec-pointer dec-slice dec-map
jsonv1 O O O ? O O
jsonv2 O O O X O O
sonicjson O O O ? O O
gojson X X X X X X
jsoniter O O O X O O
sonnetjson O O O X O O
segjson X X X X X X

while this is a minor bug, it's better to not have stackoverflows than having it. What do you think about adding an extra error to these libraries to prevent this?

dsnet commented 2 months ago

IIRC, the v1 code has logic to handle this in the simple case of a pointer being a reference to itself, but fails for the more general case of a cyclic pointer somewhere in the tree.

We should fix this in v2, but should fix the general case, but not the one-off special case. However, since this arises very rarely in practice (and isn't a security flaw since a malicious JSON input can't produce a cyclic pointer situation), the fix should ideally be low cost in memory and CPU.