Skip to content

Commit

Permalink
fix and verify compressed argument in _eckey_pubkey_serialize calls
Browse files Browse the repository at this point in the history
In several calls of the internal function
`secp256k1_eckey_pubkey_serialize`, the public API flag
`SECP256K1_EC_COMPRESSED` is passed, which is meant to be only used for
the public function `secp256k1_ec_pubkey_serialize`. It works as
intended in all of those cases (it wouldn't for `..._UNCOMPRESSED`
though), but it's still kind of a type mismatch that can't be detected
by the compiler.  To avoid cases like this in the future, a VERIFY_CHECK
is added that the `compressed` parameter needs to be either 0 or 1.
  • Loading branch information
theStack committed Oct 26, 2024
1 parent 6152622 commit 19be9a6
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 7 deletions.
3 changes: 3 additions & 0 deletions src/eckey_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "eckey.h"

#include "util.h"
#include "scalar.h"
#include "field.h"
#include "group.h"
Expand All @@ -35,6 +36,8 @@ static int secp256k1_eckey_pubkey_parse(secp256k1_ge *elem, const unsigned char
}

static int secp256k1_eckey_pubkey_serialize(secp256k1_ge *elem, unsigned char *pub, size_t *size, int compressed) {
VERIFY_CHECK(compressed == 0 || compressed == 1);

if (secp256k1_ge_is_infinity(elem)) {
return 0;
}
Expand Down
2 changes: 1 addition & 1 deletion src/modules/ecdsa_adaptor/main_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ int secp256k1_ecdsa_adaptor_recover(const secp256k1_context* ctx, unsigned char
/* We declassify non-secret enckey_expected_ge to allow using it as a
* branch point. */
secp256k1_declassify(ctx, &enckey_expected_ge, sizeof(enckey_expected_ge));
if (!secp256k1_eckey_pubkey_serialize(&enckey_expected_ge, enckey_expected33, &size, SECP256K1_EC_COMPRESSED)) {
if (!secp256k1_eckey_pubkey_serialize(&enckey_expected_ge, enckey_expected33, &size, 1)) {
/* Unreachable from tests (and other VERIFY builds) and therefore this
* branch should be ignored in test coverage analysis.
*
Expand Down
2 changes: 1 addition & 1 deletion src/modules/musig/session_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ int secp256k1_musig_nonce_gen(const secp256k1_context* ctx, secp256k1_musig_secn
if (!secp256k1_pubkey_load(ctx, &pk, pubkey)) {
return 0;
}
pk_serialize_success = secp256k1_eckey_pubkey_serialize(&pk, pk_ser, &pk_ser_len, SECP256K1_EC_COMPRESSED);
pk_serialize_success = secp256k1_eckey_pubkey_serialize(&pk, pk_ser, &pk_ser_len, 1);

#ifdef VERIFY
/* A pubkey cannot be the point at infinity */
Expand Down
8 changes: 4 additions & 4 deletions src/modules/whitelist/whitelist_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ static int secp256k1_whitelist_hash_pubkey(secp256k1_scalar* output, secp256k1_g
secp256k1_ge_set_gej(&ge, pubkey);

secp256k1_sha256_initialize(&sha);
if (!secp256k1_eckey_pubkey_serialize(&ge, c, &size, SECP256K1_EC_COMPRESSED)) {
if (!secp256k1_eckey_pubkey_serialize(&ge, c, &size, 1)) {
return 0;
}
secp256k1_sha256_write(&sha, c, size);
Expand Down Expand Up @@ -94,7 +94,7 @@ static int secp256k1_whitelist_compute_keys_and_message(const secp256k1_context*
secp256k1_pubkey_load(ctx, &subkey_ge, sub_pubkey);

/* commit to sub-key */
if (!secp256k1_eckey_pubkey_serialize(&subkey_ge, c, &size, SECP256K1_EC_COMPRESSED)) {
if (!secp256k1_eckey_pubkey_serialize(&subkey_ge, c, &size, 1)) {
return 0;
}
secp256k1_sha256_write(&sha, c, size);
Expand All @@ -105,12 +105,12 @@ static int secp256k1_whitelist_compute_keys_and_message(const secp256k1_context*

/* commit to fixed keys */
secp256k1_pubkey_load(ctx, &offline_ge, &offline_pubkeys[i]);
if (!secp256k1_eckey_pubkey_serialize(&offline_ge, c, &size, SECP256K1_EC_COMPRESSED)) {
if (!secp256k1_eckey_pubkey_serialize(&offline_ge, c, &size, 1)) {
return 0;
}
secp256k1_sha256_write(&sha, c, size);
secp256k1_pubkey_load(ctx, &online_ge, &online_pubkeys[i]);
if (!secp256k1_eckey_pubkey_serialize(&online_ge, c, &size, SECP256K1_EC_COMPRESSED)) {
if (!secp256k1_eckey_pubkey_serialize(&online_ge, c, &size, 1)) {
return 0;
}
secp256k1_sha256_write(&sha, c, size);
Expand Down
2 changes: 1 addition & 1 deletion src/secp256k1.c
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ int secp256k1_ec_pubkey_serialize(const secp256k1_context* ctx, unsigned char *o
ARG_CHECK(pubkey != NULL);
ARG_CHECK((flags & SECP256K1_FLAGS_TYPE_MASK) == SECP256K1_FLAGS_TYPE_COMPRESSION);
if (secp256k1_pubkey_load(ctx, &Q, pubkey)) {
ret = secp256k1_eckey_pubkey_serialize(&Q, output, &len, flags & SECP256K1_FLAGS_BIT_COMPRESSION);
ret = secp256k1_eckey_pubkey_serialize(&Q, output, &len, !!(flags & SECP256K1_FLAGS_BIT_COMPRESSION));
if (ret) {
*outputlen = len;
}
Expand Down

0 comments on commit 19be9a6

Please sign in to comment.