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

crypto: ensure crypto initialization works #11614

Closed
wants to merge 4 commits into from
Closed

Conversation

bagder
Copy link
Member

@bagder bagder commented Aug 7, 2023

Make sure that context initialization during hash setup works to avoid going forward with the risk of a null pointer dereference.

Reported-by: Philippe Antoine on HackerOne

Replaces #10733

Make sure that context initialization during hash setup works to avoid
going forward with the risk of a null pointer dereference.

Reported-by: Philippe Antoine on HackerOne
lib/sha256.c Show resolved Hide resolved
{
md4_init(ctx);
return CURLE_OK;
return 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielgustafsson I've changed MD4_Init prototype because it has to match backends that provide that function such as OpenSSL. See here

Another way to do it would be the way we do for md5 and sha256 where for every backend we define a CURLcode my_md4_init that either wraps MD4_Init or does whatever the alternative MD4 init procedure.

Also, I am not sure about this one above, I cannot find anything about GnuTLS md4_init in documentation. If it returns a code then this will need to be changed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GnuTLS uses nettle for crypto. nettle's md4_init function does not return anything:

https://git.lysator.liu.se/nettle/nettle/-/blob/master/md4.c#L58-74

@bagder
Copy link
Member Author

bagder commented Aug 8, 2023

Thanks for the awesome assistance here @jay!

@bagder bagder closed this in 22eb989 Aug 8, 2023
@bagder bagder deleted the bagder/sha_init branch August 8, 2023 08:44
@catenacyber
Copy link

Thanks Daniel, this was merged, right ?

@bagder
Copy link
Member Author

bagder commented Aug 10, 2023

Correct, "closed this in 22eb989" shows the commit which closed this PR when it was merged.

ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023
Make sure that context initialization during hash setup works to avoid
going forward with the risk of a null pointer dereference.

Reported-by: Philippe Antoine on HackerOne
Assisted-by: Jay Satiro
Assisted-by: Daniel Stenberg

Closes curl#11614
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

4 participants