nspcc-dev / neofs-sdk-go

Go implementation of NeoFS SDK
Apache License 2.0
6 stars 14 forks source link

client: No one checks whether the returned container matches its identifier #587

Open cthulhu-rider opened 3 months ago

cthulhu-rider commented 3 months ago

current implementation of container reader does not check resulting container against requested ID which is essentially its checksum. Thus, malicious/buggy server can return a correct but different container, and the app is unlikely to double-check it. This can lead to unexpected or even fatal consequences

choosing b/w alternatives:

  1. leave behavior as it is, add a warning to the docs
  2. make client to always check
  3. make client to check and add option to skip the check

i prefer 2 cuz, with the possible exception of testing, verified information is always expected. The need to receive information from the server “as is” may arise, but such specifics fit better into the option (i.e. implement 3, but when it'll really needed)


P.S. the same needs to be done for objects

roman-khimov commented 3 months ago

On one hand, self-checking OID/CID is a powerful thing, you can know you've got the right thing. But on the other consider blockchain interactions --- RPC node can technically return us any kind of data about balance, height, notifications, state. The only thing you can really check is blocks. And transactions included there. But what you really are interested in is state and you can only trust the node to return it to you correctly unless you have some kind of MPT verification.

One thing I'm thinking about is having better in-contract structures for containers. So that they'd be independent of protobuf serialization and potentially could even be changed. This would break this check immediately. But it'd be a powerful thing at the same time.

I'd rather just do 1 here.

carpawell commented 3 months ago

That is not the Client's responsibility IMO.