From f4edfc758142d6e100ca5d086126bf532b8a7020 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Thu, 30 Jul 2020 12:25:59 +0300 Subject: [PATCH 1/2] Improve consistency for NULL arguments in the public interface --- include/secp256k1.h | 95 ++++++++++++++++---------------- include/secp256k1_ecdh.h | 15 ++--- include/secp256k1_extrakeys.h | 82 ++++++++++++--------------- include/secp256k1_preallocated.h | 10 ++-- include/secp256k1_recovery.h | 35 ++++++------ include/secp256k1_schnorrsig.h | 10 ++-- 6 files changed, 118 insertions(+), 129 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 7be7fd5723..26c19a6011 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -226,7 +226,7 @@ SECP256K1_API secp256k1_context* secp256k1_context_create( * memory allocation entirely, see the functions in secp256k1_preallocated.h. * * Returns: a newly created context object. - * Args: ctx: an existing context to copy (cannot be NULL) + * Args: ctx: an existing context to copy */ SECP256K1_API secp256k1_context* secp256k1_context_clone( const secp256k1_context* ctx @@ -278,11 +278,11 @@ SECP256K1_API void secp256k1_context_destroy( * fails. In this case, the corresponding default handler will be called with * the data pointer argument set to NULL. * - * Args: ctx: an existing context object (cannot be NULL) + * Args: ctx: an existing context object. * In: fun: a pointer to a function to call when an illegal argument is * passed to the API, taking a message and an opaque pointer. * (NULL restores the default handler.) - * data: the opaque pointer to pass to fun above. + * data: the opaque pointer to pass to fun above, must be NULL for the default handler. * * See also secp256k1_context_set_error_callback. */ @@ -302,12 +302,12 @@ SECP256K1_API void secp256k1_context_set_illegal_callback( * for that). After this callback returns, anything may happen, including * crashing. * - * Args: ctx: an existing context object (cannot be NULL) + * Args: ctx: an existing context object. * In: fun: a pointer to a function to call when an internal error occurs, * taking a message and an opaque pointer (NULL restores the * default handler, see secp256k1_context_set_illegal_callback * for details). - * data: the opaque pointer to pass to fun above. + * data: the opaque pointer to pass to fun above, must be NULL for the default handler. * * See also secp256k1_context_set_illegal_callback. */ @@ -320,7 +320,7 @@ SECP256K1_API void secp256k1_context_set_error_callback( /** Create a secp256k1 scratch space object. * * Returns: a newly created scratch space. - * Args: ctx: an existing context object (cannot be NULL) + * Args: ctx: an existing context object. * In: size: amount of memory to be available as scratch space. Some extra * (<100 bytes) will be allocated for extra accounting. */ @@ -480,8 +480,8 @@ SECP256K1_API int secp256k1_ecdsa_signature_serialize_compact( * Returns: 1: correct signature * 0: incorrect or unparseable signature * Args: ctx: a secp256k1 context object, initialized for verification. - * In: sig: the signature being verified (cannot be NULL) - * msghash32: the 32-byte message hash being verified (cannot be NULL). + * In: sig: the signature being verified. + * msghash32: the 32-byte message hash being verified. * The verifier must make sure to apply a cryptographic * hash function to the message by itself and not accept an * msghash32 value directly. Otherwise, it would be easy to @@ -489,7 +489,7 @@ SECP256K1_API int secp256k1_ecdsa_signature_serialize_compact( * secret key. See also * https://bitcoin.stackexchange.com/a/81116/35586 for more * background on this topic. - * pubkey: pointer to an initialized public key to verify with (cannot be NULL) + * pubkey: pointer to an initialized public key to verify with. * * To avoid accepting malleable signatures, only ECDSA signatures in lower-S * form are accepted. @@ -515,8 +515,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdsa_verify( * or copy if the input was already normalized. (can be NULL if * you're only interested in whether the input was already * normalized). - * In: sigin: a pointer to a signature to check/normalize (cannot be NULL, - * can be identical to sigout) + * In: sigin: a pointer to a signature to check/normalize (can be identical to sigout) * * With ECDSA a third-party can forge a second distinct signature of the same * message, given a single initial signature, but without knowing the key. This @@ -568,12 +567,16 @@ SECP256K1_API extern const secp256k1_nonce_function secp256k1_nonce_function_def * * Returns: 1: signature created * 0: the nonce generation function failed, or the secret key was invalid. - * Args: ctx: pointer to a context object, initialized for signing (cannot be NULL) - * Out: sig: pointer to an array where the signature will be placed (cannot be NULL) - * In: msghash32: the 32-byte message hash being signed (cannot be NULL) - * seckey: pointer to a 32-byte secret key (cannot be NULL) - * noncefp: pointer to a nonce generation function. If NULL, secp256k1_nonce_function_default is used - * ndata: pointer to arbitrary data used by the nonce generation function (can be NULL) + * Args: ctx: pointer to a context object, initialized for signing. + * Out: sig: pointer to an array where the signature will be placed. + * In: msghash32: the 32-byte message hash being signed. + * seckey: pointer to a 32-byte secret key. + * noncefp: pointer to a nonce generation function. If NULL, + * secp256k1_nonce_function_default is used. + * ndata: pointer to arbitrary data used by the nonce generation function + * (can be NULL). If it is non-NULL and + * secp256k1_nonce_function_default is used, then ndata must be a + * pointer to 32-bytes of additional data. * * The created signature is always in lower-S form. See * secp256k1_ecdsa_signature_normalize for more details. @@ -596,8 +599,8 @@ SECP256K1_API int secp256k1_ecdsa_sign( * * Returns: 1: secret key is valid * 0: secret key is invalid - * Args: ctx: pointer to a context object (cannot be NULL) - * In: seckey: pointer to a 32-byte secret key (cannot be NULL) + * Args: ctx: pointer to a context object. + * In: seckey: pointer to a 32-byte secret key. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_verify( const secp256k1_context* ctx, @@ -606,11 +609,11 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_verify( /** Compute the public key for a secret key. * - * Returns: 1: secret was valid, public key stores - * 0: secret was invalid, try again - * Args: ctx: pointer to a context object, initialized for signing (cannot be NULL) - * Out: pubkey: pointer to the created public key (cannot be NULL) - * In: seckey: pointer to a 32-byte secret key (cannot be NULL) + * Returns: 1: secret was valid, public key stores. + * 0: secret was invalid, try again. + * Args: ctx: pointer to a context object, initialized for signing. + * Out: pubkey: pointer to the created public key. + * In: seckey: pointer to a 32-byte secret key. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_create( const secp256k1_context* ctx, @@ -626,8 +629,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_create( * In/Out: seckey: pointer to the 32-byte secret key to be negated. If the * secret key is invalid according to * secp256k1_ec_seckey_verify, this function returns 0 and - * seckey will be set to some unspecified value. (cannot be - * NULL) + * seckey will be set to some unspecified value. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_negate( const secp256k1_context* ctx, @@ -645,7 +647,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_negate( * * Returns: 1 always * Args: ctx: pointer to a context object - * In/Out: pubkey: pointer to the public key to be negated (cannot be NULL) + * In/Out: pubkey: pointer to the public key to be negated. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_negate( const secp256k1_context* ctx, @@ -657,15 +659,15 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_negate( * Returns: 0 if the arguments are invalid or the resulting secret key would be * invalid (only when the tweak is the negation of the secret key). 1 * otherwise. - * Args: ctx: pointer to a context object (cannot be NULL). + * Args: ctx: pointer to a context object. * In/Out: seckey: pointer to a 32-byte secret key. If the secret key is * invalid according to secp256k1_ec_seckey_verify, this * function returns 0. seckey will be set to some unspecified - * value if this function returns 0. (cannot be NULL) + * value if this function returns 0. * In: tweak32: pointer to a 32-byte tweak. If the tweak is invalid according to * secp256k1_ec_seckey_verify, this function returns 0. For * uniformly random 32-byte arrays the chance of being invalid - * is negligible (around 1 in 2^128) (cannot be NULL). + * is negligible (around 1 in 2^128). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_tweak_add( const secp256k1_context* ctx, @@ -686,14 +688,13 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_add( * Returns: 0 if the arguments are invalid or the resulting public key would be * invalid (only when the tweak is the negation of the corresponding * secret key). 1 otherwise. - * Args: ctx: pointer to a context object initialized for validation - * (cannot be NULL). + * Args: ctx: pointer to a context object initialized for validation. * In/Out: pubkey: pointer to a public key object. pubkey will be set to an - * invalid value if this function returns 0 (cannot be NULL). + * invalid value if this function returns 0. * In: tweak32: pointer to a 32-byte tweak. If the tweak is invalid according to * secp256k1_ec_seckey_verify, this function returns 0. For * uniformly random 32-byte arrays the chance of being invalid - * is negligible (around 1 in 2^128) (cannot be NULL). + * is negligible (around 1 in 2^128). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( const secp256k1_context* ctx, @@ -704,15 +705,15 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( /** Tweak a secret key by multiplying it by a tweak. * * Returns: 0 if the arguments are invalid. 1 otherwise. - * Args: ctx: pointer to a context object (cannot be NULL). + * Args: ctx: pointer to a context object. * In/Out: seckey: pointer to a 32-byte secret key. If the secret key is * invalid according to secp256k1_ec_seckey_verify, this * function returns 0. seckey will be set to some unspecified - * value if this function returns 0. (cannot be NULL) + * value if this function returns 0. * In: tweak32: pointer to a 32-byte tweak. If the tweak is invalid according to * secp256k1_ec_seckey_verify, this function returns 0. For * uniformly random 32-byte arrays the chance of being invalid - * is negligible (around 1 in 2^128) (cannot be NULL). + * is negligible (around 1 in 2^128). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_tweak_mul( const secp256k1_context* ctx, @@ -731,14 +732,13 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_mul( /** Tweak a public key by multiplying it by a tweak value. * * Returns: 0 if the arguments are invalid. 1 otherwise. - * Args: ctx: pointer to a context object initialized for validation - * (cannot be NULL). + * Args: ctx: pointer to a context object initialized for validation. * In/Out: pubkey: pointer to a public key object. pubkey will be set to an - * invalid value if this function returns 0 (cannot be NULL). + * invalid value if this function returns 0. * In: tweak32: pointer to a 32-byte tweak. If the tweak is invalid according to * secp256k1_ec_seckey_verify, this function returns 0. For * uniformly random 32-byte arrays the chance of being invalid - * is negligible (around 1 in 2^128) (cannot be NULL). + * is negligible (around 1 in 2^128). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_mul( const secp256k1_context* ctx, @@ -749,7 +749,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_mul( /** Updates the context randomization to protect against side-channel leakage. * Returns: 1: randomization successfully updated or nothing to randomize * 0: error - * Args: ctx: pointer to a context object (cannot be NULL) + * Args: ctx: pointer to a context object. * In: seed32: pointer to a 32-byte random seed (NULL resets to initial state) * * While secp256k1 code is written to be constant-time no matter what secret @@ -780,18 +780,17 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_context_randomize( * * Returns: 1: the sum of the public keys is valid. * 0: the sum of the public keys is not valid. - * Args: ctx: pointer to a context object - * Out: out: pointer to a public key object for placing the resulting public key - * (cannot be NULL) - * In: ins: pointer to array of pointers to public keys (cannot be NULL) - * n: the number of public keys to add together (must be at least 1) + * Args: ctx: pointer to a context object. + * Out: out: pointer to a public key object for placing the resulting public key. + * In: ins: pointer to array of pointers to public keys. + * n: the number of public keys to add together (must be at least 1). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_combine( const secp256k1_context* ctx, secp256k1_pubkey *out, const secp256k1_pubkey * const * ins, size_t n -) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); /** Compute a tagged hash as defined in BIP-340. * diff --git a/include/secp256k1_ecdh.h b/include/secp256k1_ecdh.h index 4058e9c043..c8577984b1 100644 --- a/include/secp256k1_ecdh.h +++ b/include/secp256k1_ecdh.h @@ -37,14 +37,15 @@ SECP256K1_API extern const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_func * * Returns: 1: exponentiation was successful * 0: scalar was invalid (zero or overflow) or hashfp returned 0 - * Args: ctx: pointer to a context object (cannot be NULL) - * Out: output: pointer to an array to be filled by hashfp - * In: pubkey: a pointer to a secp256k1_pubkey containing an - * initialized public key - * seckey: a 32-byte scalar with which to multiply the point - * hashfp: pointer to a hash function. If NULL, secp256k1_ecdh_hash_function_sha256 is used - * (in which case, 32 bytes will be written to output) + * Args: ctx: pointer to a context object. + * Out: output: pointer to an array to be filled by hashfp. + * In: pubkey: a pointer to a secp256k1_pubkey containing an initialized public key. + * seckey: a 32-byte scalar with which to multiply the point. + * hashfp: pointer to a hash function. If NULL, + * secp256k1_ecdh_hash_function_sha256 is used + * (in which case, 32 bytes will be written to output). * data: arbitrary data pointer that is passed through to hashfp + * (can be NULL for secp256k1_ecdh_hash_function_sha256). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdh( const secp256k1_context* ctx, diff --git a/include/secp256k1_extrakeys.h b/include/secp256k1_extrakeys.h index 0a37fb6b9d..a64d561b60 100644 --- a/include/secp256k1_extrakeys.h +++ b/include/secp256k1_extrakeys.h @@ -39,11 +39,10 @@ typedef struct { * Returns: 1 if the public key was fully valid. * 0 if the public key could not be parsed or is invalid. * - * Args: ctx: a secp256k1 context object (cannot be NULL). + * Args: ctx: a secp256k1 context object. * Out: pubkey: pointer to a pubkey object. If 1 is returned, it is set to a * parsed version of input. If not, it's set to an invalid value. - * (cannot be NULL). - * In: input32: pointer to a serialized xonly_pubkey (cannot be NULL) + * In: input32: pointer to a serialized xonly_pubkey. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_parse( const secp256k1_context* ctx, @@ -55,11 +54,9 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_parse( * * Returns: 1 always. * - * Args: ctx: a secp256k1 context object (cannot be NULL). - * Out: output32: a pointer to a 32-byte array to place the serialized key in - * (cannot be NULL). - * In: pubkey: a pointer to a secp256k1_xonly_pubkey containing an - * initialized public key (cannot be NULL). + * Args: ctx: a secp256k1 context object. + * Out: output32: a pointer to a 32-byte array to place the serialized key in. + * In: pubkey: a pointer to a secp256k1_xonly_pubkey containing an initialized public key. */ SECP256K1_API int secp256k1_xonly_pubkey_serialize( const secp256k1_context* ctx, @@ -87,13 +84,12 @@ SECP256K1_API int secp256k1_xonly_pubkey_cmp( * Returns: 1 if the public key was successfully converted * 0 otherwise * - * Args: ctx: pointer to a context object (cannot be NULL) - * Out: xonly_pubkey: pointer to an x-only public key object for placing the - * converted public key (cannot be NULL) - * pk_parity: pointer to an integer that will be set to 1 if the point - * encoded by xonly_pubkey is the negation of the pubkey and - * set to 0 otherwise. (can be NULL) - * In: pubkey: pointer to a public key that is converted (cannot be NULL) + * Args: ctx: pointer to a context object. + * Out: xonly_pubkey: pointer to an x-only public key object for placing the converted public key. + * pk_parity: Ignored if NULL. Otherwise, pointer to an integer that + * will be set to 1 if the point encoded by xonly_pubkey is + * the negation of the pubkey and set to 0 otherwise. + * In: pubkey: pointer to a public key that is converted. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_from_pubkey( const secp256k1_context* ctx, @@ -113,18 +109,14 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_from_pubke * invalid (only when the tweak is the negation of the corresponding * secret key). 1 otherwise. * - * Args: ctx: pointer to a context object initialized for verification - * (cannot be NULL) + * Args: ctx: pointer to a context object initialized for verification. * Out: output_pubkey: pointer to a public key to store the result. Will be set - * to an invalid value if this function returns 0 (cannot - * be NULL) + * to an invalid value if this function returns 0. * In: internal_pubkey: pointer to an x-only pubkey to apply the tweak to. - * (cannot be NULL). * tweak32: pointer to a 32-byte tweak. If the tweak is invalid * according to secp256k1_ec_seckey_verify, this function * returns 0. For uniformly random 32-byte arrays the - * chance of being invalid is negligible (around 1 in - * 2^128) (cannot be NULL). + * chance of being invalid is negligible (around 1 in 2^128). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_tweak_add( const secp256k1_context* ctx, @@ -146,17 +138,15 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_tweak_add( * * Returns: 0 if the arguments are invalid or the tweaked pubkey is not the * result of tweaking the internal_pubkey with tweak32. 1 otherwise. - * Args: ctx: pointer to a context object initialized for verification - * (cannot be NULL) - * In: tweaked_pubkey32: pointer to a serialized xonly_pubkey (cannot be NULL) + * Args: ctx: pointer to a context object initialized for verification. + * In: tweaked_pubkey32: pointer to a serialized xonly_pubkey. * tweaked_pk_parity: the parity of the tweaked pubkey (whose serialization * is passed in as tweaked_pubkey32). This must match the * pk_parity value that is returned when calling * secp256k1_xonly_pubkey with the tweaked pubkey, or * this function will fail. - * internal_pubkey: pointer to an x-only public key object to apply the - * tweak to (cannot be NULL) - * tweak32: pointer to a 32-byte tweak (cannot be NULL) + * internal_pubkey: pointer to an x-only public key object to apply the tweak to. + * tweak32: pointer to a 32-byte tweak. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_tweak_add_check( const secp256k1_context* ctx, @@ -170,9 +160,9 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_tweak_add_ * * Returns: 1: secret was valid, keypair is ready to use * 0: secret was invalid, try again with a different secret - * Args: ctx: pointer to a context object, initialized for signing (cannot be NULL) - * Out: keypair: pointer to the created keypair (cannot be NULL) - * In: seckey: pointer to a 32-byte secret key (cannot be NULL) + * Args: ctx: pointer to a context object, initialized for signing. + * Out: keypair: pointer to the created keypair. + * In: seckey: pointer to a 32-byte secret key. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_create( const secp256k1_context* ctx, @@ -183,9 +173,9 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_create( /** Get the secret key from a keypair. * * Returns: 0 if the arguments are invalid. 1 otherwise. - * Args: ctx: pointer to a context object (cannot be NULL) - * Out: seckey: pointer to a 32-byte buffer for the secret key (cannot be NULL) - * In: keypair: pointer to a keypair (cannot be NULL) + * Args: ctx: pointer to a context object. + * Out: seckey: pointer to a 32-byte buffer for the secret key. + * In: keypair: pointer to a keypair. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_sec( const secp256k1_context* ctx, @@ -196,11 +186,10 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_sec( /** Get the public key from a keypair. * * Returns: 0 if the arguments are invalid. 1 otherwise. - * Args: ctx: pointer to a context object (cannot be NULL) + * Args: ctx: pointer to a context object. * Out: pubkey: pointer to a pubkey object. If 1 is returned, it is set to * the keypair public key. If not, it's set to an invalid value. - * (cannot be NULL) - * In: keypair: pointer to a keypair (cannot be NULL) + * In: keypair: pointer to a keypair. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_pub( const secp256k1_context* ctx, @@ -214,14 +203,13 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_pub( * secp256k1_xonly_pubkey_from_pubkey. * * Returns: 0 if the arguments are invalid. 1 otherwise. - * Args: ctx: pointer to a context object (cannot be NULL) + * Args: ctx: pointer to a context object. * Out: pubkey: pointer to an xonly_pubkey object. If 1 is returned, it is set * to the keypair public key after converting it to an - * xonly_pubkey. If not, it's set to an invalid value (cannot be - * NULL). - * pk_parity: pointer to an integer that will be set to the pk_parity - * argument of secp256k1_xonly_pubkey_from_pubkey (can be NULL). - * In: keypair: pointer to a keypair (cannot be NULL) + * xonly_pubkey. If not, it's set to an invalid value. + * pk_parity: Ignored if NULL. Otherwise, pointer to an integer that will be set to the + * pk_parity argument of secp256k1_xonly_pubkey_from_pubkey. + * In: keypair: pointer to a keypair. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_xonly_pub( const secp256k1_context* ctx, @@ -241,15 +229,13 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_xonly_pub( * invalid (only when the tweak is the negation of the keypair's * secret key). 1 otherwise. * - * Args: ctx: pointer to a context object initialized for verification - * (cannot be NULL) + * Args: ctx: pointer to a context object initialized for verification. * In/Out: keypair: pointer to a keypair to apply the tweak to. Will be set to - * an invalid value if this function returns 0 (cannot be - * NULL). + * an invalid value if this function returns 0. * In: tweak32: pointer to a 32-byte tweak. If the tweak is invalid according * to secp256k1_ec_seckey_verify, this function returns 0. For * uniformly random 32-byte arrays the chance of being invalid - * is negligible (around 1 in 2^128) (cannot be NULL). + * is negligible (around 1 in 2^128). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_xonly_tweak_add( const secp256k1_context* ctx, diff --git a/include/secp256k1_preallocated.h b/include/secp256k1_preallocated.h index a9ae15d5ae..1ab8b7d6c7 100644 --- a/include/secp256k1_preallocated.h +++ b/include/secp256k1_preallocated.h @@ -55,7 +55,7 @@ SECP256K1_API size_t secp256k1_context_preallocated_size( * Returns: a newly created context object. * In: prealloc: a pointer to a rewritable contiguous block of memory of * size at least secp256k1_context_preallocated_size(flags) - * bytes, as detailed above (cannot be NULL) + * bytes, as detailed above. * flags: which parts of the context to initialize. * * See also secp256k1_context_randomize (in secp256k1.h) @@ -70,7 +70,7 @@ SECP256K1_API secp256k1_context* secp256k1_context_preallocated_create( * caller-provided memory. * * Returns: the required size of the caller-provided memory block. - * In: ctx: an existing context to copy (cannot be NULL) + * In: ctx: an existing context to copy. */ SECP256K1_API size_t secp256k1_context_preallocated_clone_size( const secp256k1_context* ctx @@ -87,10 +87,10 @@ SECP256K1_API size_t secp256k1_context_preallocated_clone_size( * secp256k1_context_preallocated_create for details. * * Returns: a newly created context object. - * Args: ctx: an existing context to copy (cannot be NULL) + * Args: ctx: an existing context to copy. * In: prealloc: a pointer to a rewritable contiguous block of memory of * size at least secp256k1_context_preallocated_size(flags) - * bytes, as detailed above (cannot be NULL) + * bytes, as detailed above. */ SECP256K1_API secp256k1_context* secp256k1_context_preallocated_clone( const secp256k1_context* ctx, @@ -115,7 +115,7 @@ SECP256K1_API secp256k1_context* secp256k1_context_preallocated_clone( * * Args: ctx: an existing context to destroy, constructed using * secp256k1_context_preallocated_create or - * secp256k1_context_preallocated_clone (cannot be NULL) + * secp256k1_context_preallocated_clone. */ SECP256K1_API void secp256k1_context_preallocated_destroy( secp256k1_context* ctx diff --git a/include/secp256k1_recovery.h b/include/secp256k1_recovery.h index aa16532ce8..0e2847db96 100644 --- a/include/secp256k1_recovery.h +++ b/include/secp256k1_recovery.h @@ -43,8 +43,9 @@ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_parse_compact( /** Convert a recoverable signature into a normal signature. * * Returns: 1 - * Out: sig: a pointer to a normal signature (cannot be NULL). - * In: sigin: a pointer to a recoverable signature (cannot be NULL). + * Args: ctx: a secp256k1 context object. + * Out: sig: a pointer to a normal signature. + * In: sigin: a pointer to a recoverable signature. */ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_convert( const secp256k1_context* ctx, @@ -55,10 +56,10 @@ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_convert( /** Serialize an ECDSA signature in compact format (64 bytes + recovery id). * * Returns: 1 - * Args: ctx: a secp256k1 context object - * Out: output64: a pointer to a 64-byte array of the compact signature (cannot be NULL) - * recid: a pointer to an integer to hold the recovery id (can be NULL). - * In: sig: a pointer to an initialized signature object (cannot be NULL) + * Args: ctx: a secp256k1 context object. + * Out: output64: a pointer to a 64-byte array of the compact signature. + * recid: a pointer to an integer to hold the recovery id. + * In: sig: a pointer to an initialized signature object. */ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_serialize_compact( const secp256k1_context* ctx, @@ -71,12 +72,14 @@ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_serialize_compact( * * Returns: 1: signature created * 0: the nonce generation function failed, or the secret key was invalid. - * Args: ctx: pointer to a context object, initialized for signing (cannot be NULL) - * Out: sig: pointer to an array where the signature will be placed (cannot be NULL) - * In: msghash32: the 32-byte message hash being signed (cannot be NULL) - * seckey: pointer to a 32-byte secret key (cannot be NULL) - * noncefp: pointer to a nonce generation function. If NULL, secp256k1_nonce_function_default is used - * ndata: pointer to arbitrary data used by the nonce generation function (can be NULL) + * Args: ctx: pointer to a context object, initialized for signing. + * Out: sig: pointer to an array where the signature will be placed. + * In: msghash32: the 32-byte message hash being signed. + * seckey: pointer to a 32-byte secret key. + * noncefp: pointer to a nonce generation function. If NULL, + * secp256k1_nonce_function_default is used. + * ndata: pointer to arbitrary data used by the nonce generation function + * (can be NULL for secp256k1_nonce_function_default). */ SECP256K1_API int secp256k1_ecdsa_sign_recoverable( const secp256k1_context* ctx, @@ -91,10 +94,10 @@ SECP256K1_API int secp256k1_ecdsa_sign_recoverable( * * Returns: 1: public key successfully recovered (which guarantees a correct signature). * 0: otherwise. - * Args: ctx: pointer to a context object, initialized for verification (cannot be NULL) - * Out: pubkey: pointer to the recovered public key (cannot be NULL) - * In: sig: pointer to initialized signature that supports pubkey recovery (cannot be NULL) - * msghash32: the 32-byte message hash assumed to be signed (cannot be NULL) + * Args: ctx: pointer to a context object, initialized for verification. + * Out: pubkey: pointer to the recovered public key. + * In: sig: pointer to initialized signature that supports pubkey recovery. + * msghash32: the 32-byte message hash assumed to be signed. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdsa_recover( const secp256k1_context* ctx, diff --git a/include/secp256k1_schnorrsig.h b/include/secp256k1_schnorrsig.h index d68bba62cc..c42cb10f2b 100644 --- a/include/secp256k1_schnorrsig.h +++ b/include/secp256k1_schnorrsig.h @@ -106,10 +106,10 @@ typedef struct { * signatures from being valid in multiple contexts by accident. * * Returns 1 on success, 0 on failure. - * Args: ctx: pointer to a context object, initialized for signing (cannot be NULL) - * Out: sig64: pointer to a 64-byte array to store the serialized signature (cannot be NULL) - * In: msg32: the 32-byte message being signed (cannot be NULL) - * keypair: pointer to an initialized keypair (cannot be NULL) + * Args: ctx: pointer to a context object, initialized for signing. + * Out: sig64: pointer to a 64-byte array to store the serialized signature. + * In: msg32: the 32-byte message being signed. + * keypair: pointer to an initialized keypair. * aux_rand32: 32 bytes of fresh randomness. While recommended to provide * this, it is only supplemental to security and can be NULL. See * BIP-340 "Default Signing" for a full explanation of this @@ -150,7 +150,7 @@ SECP256K1_API int secp256k1_schnorrsig_sign_custom( * Returns: 1: correct signature * 0: incorrect signature * Args: ctx: a secp256k1 context object, initialized for verification. - * In: sig64: pointer to the 64-byte signature to verify (cannot be NULL) + * In: sig64: pointer to the 64-byte signature to verify. * msg: the message being verified. Can only be NULL if msglen is 0. * msglen: length of the message * pubkey: pointer to an x-only public key to verify with (cannot be NULL) From adec5a16383f1704d80d7c767b2a65d9221cee08 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Thu, 30 Jul 2020 12:26:28 +0300 Subject: [PATCH 2/2] Add missing null check for ctx and input keys in the public API --- include/secp256k1.h | 2 +- include/secp256k1_preallocated.h | 2 +- src/modules/recovery/main_impl.h | 6 +++--- src/secp256k1.c | 2 ++ 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 26c19a6011..576953f49d 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -247,7 +247,7 @@ SECP256K1_API secp256k1_context* secp256k1_context_clone( */ SECP256K1_API void secp256k1_context_destroy( secp256k1_context* ctx -); +) SECP256K1_ARG_NONNULL(1); /** Set a callback function to be called when an illegal argument is passed to * an API call. It will only trigger for violations that are mentioned diff --git a/include/secp256k1_preallocated.h b/include/secp256k1_preallocated.h index 1ab8b7d6c7..d2d9014f02 100644 --- a/include/secp256k1_preallocated.h +++ b/include/secp256k1_preallocated.h @@ -119,7 +119,7 @@ SECP256K1_API secp256k1_context* secp256k1_context_preallocated_clone( */ SECP256K1_API void secp256k1_context_preallocated_destroy( secp256k1_context* ctx -); +) SECP256K1_ARG_NONNULL(1); #ifdef __cplusplus } diff --git a/src/modules/recovery/main_impl.h b/src/modules/recovery/main_impl.h index 9e19f2a2dc..3f29570198 100644 --- a/src/modules/recovery/main_impl.h +++ b/src/modules/recovery/main_impl.h @@ -40,7 +40,7 @@ int secp256k1_ecdsa_recoverable_signature_parse_compact(const secp256k1_context* int ret = 1; int overflow = 0; - (void)ctx; + VERIFY_CHECK(ctx != NULL); ARG_CHECK(sig != NULL); ARG_CHECK(input64 != NULL); ARG_CHECK(recid >= 0 && recid <= 3); @@ -60,7 +60,7 @@ int secp256k1_ecdsa_recoverable_signature_parse_compact(const secp256k1_context* int secp256k1_ecdsa_recoverable_signature_serialize_compact(const secp256k1_context* ctx, unsigned char *output64, int *recid, const secp256k1_ecdsa_recoverable_signature* sig) { secp256k1_scalar r, s; - (void)ctx; + VERIFY_CHECK(ctx != NULL); ARG_CHECK(output64 != NULL); ARG_CHECK(sig != NULL); ARG_CHECK(recid != NULL); @@ -75,7 +75,7 @@ int secp256k1_ecdsa_recoverable_signature_convert(const secp256k1_context* ctx, secp256k1_scalar r, s; int recid; - (void)ctx; + VERIFY_CHECK(ctx != NULL); ARG_CHECK(sig != NULL); ARG_CHECK(sigin != NULL); diff --git a/src/secp256k1.c b/src/secp256k1.c index 9908cab864..adff3ae1be 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -771,6 +771,7 @@ int secp256k1_ec_pubkey_combine(const secp256k1_context* ctx, secp256k1_pubkey * secp256k1_gej Qj; secp256k1_ge Q; + VERIFY_CHECK(ctx != NULL); ARG_CHECK(pubnonce != NULL); memset(pubnonce, 0, sizeof(*pubnonce)); ARG_CHECK(n >= 1); @@ -779,6 +780,7 @@ int secp256k1_ec_pubkey_combine(const secp256k1_context* ctx, secp256k1_pubkey * secp256k1_gej_set_infinity(&Qj); for (i = 0; i < n; i++) { + ARG_CHECK(pubnonces[i] != NULL); secp256k1_pubkey_load(ctx, &Q, pubnonces[i]); secp256k1_gej_add_ge(&Qj, &Qj, &Q); }