Skip to content

Verify non-null data arg in ellswift xdh_hash_function_prefix#1806

Open
furszy wants to merge 1 commit intobitcoin-core:masterfrom
furszy:2026_ellswift_xdh_verify_data_arg
Open

Verify non-null data arg in ellswift xdh_hash_function_prefix#1806
furszy wants to merge 1 commit intobitcoin-core:masterfrom
furszy:2026_ellswift_xdh_verify_data_arg

Conversation

@furszy
Copy link
Member

@furszy furszy commented Jan 23, 2026

The secp256k1_ellswift_xdh API has a data arg that is optional for one
of the hash functions (unused) but required for the other.

This adds a simple check to provide a clear error instead of crashing
due to an out-of-bounds memory access when the user mistakenly provides
data=NULL and selects the xdh_hash_function_prefix() function.

Note: Open to improving the arg description in the API as well. I'm just adding
this check because a better error would have saved me a few minutes.

The secp256k1_ellswift_xdh API has a `data` arg that is optional
for one of the hash functions (unused) but required for the other.

This adds a simple check to provide a clear error instead of crashing
due to an out-of-bounds memory access when the user mistakenly provides
data=NULL and selects the `xdh_hash_function_prefix()` function.
Copy link
Contributor

@sipa sipa left a comment

Choose a reason for hiding this comment

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

ACK f4708d2

@real-or-random
Copy link
Contributor

when the user mistakenly provides
data=NULL

Note that VERIFY_CHECKs are only active in the tests.

@furszy
Copy link
Member Author

furszy commented Jan 23, 2026

when the user mistakenly provides
data=NULL

Note that VERIFY_CHECKs are only active in the tests.

We can't use ARG_CHECK because ellswift_xdh_hash_function_prefix() doesn’t receive the context (for now). #1777 will solve that and let us add the proper arg check.

Update: We probably could leave this open until the other PR gets merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants