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

Does JSON allowing duplicate keys affect TUF security? #372

Closed
trishankkarthik opened this issue Sep 16, 2016 · 5 comments
Closed

Does JSON allowing duplicate keys affect TUF security? #372

trishankkarthik opened this issue Sep 16, 2016 · 5 comments

Comments

@trishankkarthik
Copy link
Contributor

Although we use objects/dictionaries/associate arrays in JSON to ensure that the TUF updater would not associate multiple values with the same key, note that this does not prevent duplicate keys from being transmitted in the JSON anyway. Furthermore, a JSON deserializer may not warn about duplicate keys. For example, in Python now:

>>> import json; d=json.loads('{"foo":"bar","foo":"baz"}'); print(d['foo'])
baz

I haven't thought through whether this has security implications, but I wonder whether this may be worth considering.

@trishankkarthik trishankkarthik added bug question security client Related to the client (updater) implementation labels Sep 16, 2016
@trishankkarthik
Copy link
Contributor Author

Key duplicates not the only issue...

@trishankatdatadog trishankatdatadog changed the title JSON deserialization does not necessarily detect duplicate keys Does JSON allowing duplicate keys affect TUF security? Dec 17, 2019
@trishankatdatadog trishankatdatadog removed bug security client Related to the client (updater) implementation labels Dec 17, 2019
@rbtcollins
Copy link

This sort of thing has led to issues in other systems in the past, in particular where two different parser implementations behave differently, it can lead to divergent behaviour. E.g. as the signatures are embedded in JSON today a MITM attacker could use this to get some TUF clients to see bar and others to see baz, presuming of course that there were more than one implementation of JSON parser in use in TUF implementations in the wild. Pinning down the exact behaviour and using an implementation with verified behaviour (per that excellent link) would be a great idea.

@trishankatdatadog
Copy link
Member

@rbtcollins Thanks!

TBF, I don't how this a JSON-specific issue. You can get the same issue in ASN.1 (never mind other security issues associated with it). I don't think there's any data interchange standard that prevents attackers from listing the same thing twice. Perhaps what you mean is that parsers or TUF clients should be careful in treating metadata with duplicates?

@rbtcollins
Copy link

I agree. I think it is a property of any serialization scheme where the behaviour of repeated keys in a hash is implementation defined. First-key, Last-key or multi-map can all be secure, but "implementors choose a strategy they prefer" much less so.

@jku
Copy link
Member

jku commented Feb 17, 2022

we could create our own object_pairs_hook to error/warn if we see duplicate keys in deserialization but I don't think this will end up on top of my TODO list ... and looks like this opinion is shared as nothing has happened here in six years.

I think I'll close this: If this was meant to be a specification issue instead of python-tuf issue, please file in https://github.com/theupdateframework/specification/issues (for the record, I think "last-key-wins" is the right choice as it tends to Just Work, but explicitly erroring out on duplicates would be doable as well)

@jku jku closed this as completed Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants