ssz: fix buffer overflows, explicit error reporting

[arnetheduck]

• fix buffer overflow in seq reading
• report errors explicitly - overflows when reading byte buffers are expected

[mratsim]

LGTM, actually that's one part of the spec that is not covered at the moment.

[arnetheduck]

How do you mean, not covered? Given the fixed code above, it's clear that the format gives enough data to implement safe parsing - all lengths are known ahead of reading the data - either from schema or explicit in the data stream, meaning there's no case where you have to keep reading an unknown amount of bytes that might lead to trouble

[mratsim]

Not covered in the sense what if we get malformatted data how to handle that gracefully.

[arnetheduck]

malformatted

drop it completely? there's really nothing else you can do. this is not a self-healing format, nor do I think it should be..

[mratsim]

Yeah of course, but at a higher level, do we continue silently, ask for new data, slash a stake, nuke a shard chain?

[arnetheduck]

Well, it's treated the same way as if the serialized data wasn't sent at all - it's equivalent from the higher level perspective - in fact, it's likely that if ever a client sends malformed data, that client should simply be dropped.

[mkalinin]

I agree with @arnetheduck. Any peer that sent malformed data once will likely do that again, cause it's either buggy or malicious. Thus, it should be immediately dropped. I don't think it will be useful to add any recommendations like that to SSZ spec or to any other spec. It looks more like an implementers decision what to do in each particular case (maybe some network monitor would like to collect statistics of malicious/buggy peers).