Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
cthulhu-rider opened this issue Jun 17, 2024 · 2 comments
Labels
bug Something isn't working I4 No visible changes S3 Minimally significant security Affects security U4 Nothing urgent

Comments

@cthulhu-rider
Copy link
Contributor

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

@cthulhu-rider cthulhu-rider added bug Something isn't working discussion Open discussion of some problem labels Jun 17, 2024
@roman-khimov
Copy link
Member

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 roman-khimov added U4 Nothing urgent S3 Minimally significant I4 No visible changes security Affects security and removed discussion Open discussion of some problem labels Jun 17, 2024
@carpawell
Copy link
Member

That is not the Client's responsibility IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working I4 No visible changes S3 Minimally significant security Affects security U4 Nothing urgent
Projects
None yet
Development

No branches or pull requests

3 participants