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.

Sign up for GitHub

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

Jump to bottom

Support configuring default salt #218

Open
brandon-leapyear opened this issue Sep 13, 2021 · 5 comments · May be fixed by #219
Open

Support configuring default salt #218

brandon-leapyear opened this issue Sep 13, 2021 · 5 comments · May be fixed by #219

Comments

@brandon-leapyear
Copy link

Blog post: https://cs-syd.eu/posts/2021-09-11-json-vulnerability

The biggest problem exposed by this blog post is that any third-party code using hash (using the default salt) that has problems with hash collisions can have malicious input constructed beforehand. The -frandom-init-seed flag helps with this at the cost of a different seed every start up.

One quick fix that can help mitigate this is to allow reading an environment variable like HASHABLE_DEFAULT_SALT with unsafePerformIO that one can set differently for their application. This has the advantage that my application can hash things with a different salt than the default, publicly available one, so an attacker attacking my application can't reuse malicious values from attacking another application. It would also be difficult to construct malicious input if my application's default salt isn't shared, and my application doesn't easily expose the default salt in any way.

Related:
This was referenced Sep 13, 2021
Open
Open
@NorfairKing
Copy link

One quick fix that can help mitigate this is to allow reading an environment variable like HASHABLE_DEFAULT_SALT with unsafePerformIO that one can set differently for their application.

This is SO hacky that we shouldn't even merge the PR, just have people use the open PR if necessary. IMO

@brandon-leapyear
Copy link
Author

brandon-leapyear commented Sep 13, 2021 •
edited

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

I disagree that it's "SO" hacky. Using environment variables to configure upstream behavior is not uncommon (SYSTEM_CERTIFICATE_PATH for x509-system or http_proxy for http-client). unsafePerformIO isn't terribly hacky if used correctly, in this case to do one-time IO setup actions.

@jappeace
Copy link

jappeace commented Sep 15, 2021 •
edited

wouldn't setting HASHABLE_RANDOM_SEED work?

Apparently, this works:

cabal new-run hashable-examples -f random-initial-seed

It doesn't give the ability for configuring it, but I think you don't want to configure it anyway, just let it be random on boot.

@NorfairKing
Copy link

@jappeace yes that's a (very short-term) workaround.

@brandon-leapyear
Copy link
Author

brandon-leapyear commented Sep 15, 2021 •
edited

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

It doesn't give the ability for configuring it, but I think you don't want to configure it anyway, just let it be random on boot.

Yes, I mentioned that in the beginning

The -frandom-init-seed flag helps with this at the cost of a different seed every start up.

But in some situations, you don't want it to be random on boot, such as when you need deterministic tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support HASHABLE_INITIAL_SEED brandon-leapyear/hashable
3 participants
@NorfairKing @jappeace @brandon-leapyear