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

New issue

Open

opened 2025-12-28 18:07:29 +00:00 by sami · 2 comments

sami commented

2025-12-28 18:07:29 +00:00

Owner

Originally created by @cthulhu-rider on GitHub (Jun 17, 2024).

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:

leave behavior as it is, add a warning to the docs
make client to always check
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

Originally created by @cthulhu-rider on GitHub (Jun 17, 2024). current implementation of container [reader](https://pkg.go.dev/github.com/nspcc-dev/neofs-sdk-go/client#Client.ContainerGet) 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

sami added the

bug

security

labels

2025-12-28 18:07:29 +00:00

sami commented

2025-12-28 18:07:29 +00:00

Author

Owner

@roman-khimov commented on GitHub (Jun 17, 2024):

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.

@roman-khimov commented on GitHub (Jun 17, 2024): 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.

sami commented

2025-12-28 18:07:29 +00:00

Author

Owner

@carpawell commented on GitHub (Jun 18, 2024):

That is not the Client's responsibility IMO.

@carpawell commented on GitHub (Jun 18, 2024): That is not the `Client`'s responsibility IMO.