Skip to content

Commit

Permalink
Require that sizeof(secp256k1_ge_storage) == 64
Browse files Browse the repository at this point in the history
This gets rid of an untested code path. Resolves #1352.

secp256k1_ge_storage is a struct with two secp256k1_fe_storage fields.
The C standard allows the compiler to add padding between the fields and
at the end of the struct, but no sane compiler in the end would do this:
The only reason to add padding is to ensure alignment, but such padding
is never necessary between two fields of the same type.

Similarly, secp256k1_fe_storage is a struct with a single array of
uintXX_t. No padding is allowed between array elements. Again, C allows
the compiler to insert padding at the end of the struct, but there's no
absolute reason to do so in this case.

For the uintXX_t itself, this guaranteed to have no padding bits, i.e.,
it's guaranteed to have exactly XX bits.

So I claim that for any existing compiler in the real world,
sizeof(secp256k1_ge_storage) == 64.
  • Loading branch information
real-or-random committed Jan 8, 2024
1 parent d0ba2ab commit e53c2d9
Showing 1 changed file with 14 additions and 25 deletions.
39 changes: 14 additions & 25 deletions src/secp256k1.c
Original file line number Diff line number Diff line change
Expand Up @@ -237,36 +237,25 @@ static SECP256K1_INLINE void secp256k1_declassify(const secp256k1_context* ctx,
}

static int secp256k1_pubkey_load(const secp256k1_context* ctx, secp256k1_ge* ge, const secp256k1_pubkey* pubkey) {
if (sizeof(secp256k1_ge_storage) == 64) {
/* When the secp256k1_ge_storage type is exactly 64 byte, use its
* representation inside secp256k1_pubkey, as conversion is very fast.
* Note that secp256k1_pubkey_save must use the same representation. */
secp256k1_ge_storage s;
memcpy(&s, &pubkey->data[0], sizeof(s));
secp256k1_ge_from_storage(ge, &s);
} else {
/* Otherwise, fall back to 32-byte big endian for X and Y. */
secp256k1_fe x, y;
ARG_CHECK(secp256k1_fe_set_b32_limit(&x, pubkey->data));
ARG_CHECK(secp256k1_fe_set_b32_limit(&y, pubkey->data + 32));
secp256k1_ge_set_xy(ge, &x, &y);
}
secp256k1_ge_storage s;

/* We require that the secp256k1_ge_storage type is exactly 64 bytes.
* This is formally not guaranteed by the C standard, but should hold on any
* sane compiler in the real world. */
STATIC_ASSERT(sizeof(secp256k1_ge_storage) == 64);
memcpy(&s, &pubkey->data[0], 64);
secp256k1_ge_from_storage(ge, &s);
ARG_CHECK(!secp256k1_fe_is_zero(&ge->x));
return 1;
}

static void secp256k1_pubkey_save(secp256k1_pubkey* pubkey, secp256k1_ge* ge) {
if (sizeof(secp256k1_ge_storage) == 64) {
secp256k1_ge_storage s;
secp256k1_ge_to_storage(&s, ge);
memcpy(&pubkey->data[0], &s, sizeof(s));
} else {
VERIFY_CHECK(!secp256k1_ge_is_infinity(ge));
secp256k1_fe_normalize_var(&ge->x);
secp256k1_fe_normalize_var(&ge->y);
secp256k1_fe_get_b32(pubkey->data, &ge->x);
secp256k1_fe_get_b32(pubkey->data + 32, &ge->y);
}
secp256k1_ge_storage s;

STATIC_ASSERT(sizeof(secp256k1_ge_storage) == 64);
VERIFY_CHECK(!secp256k1_ge_is_infinity(ge));
secp256k1_ge_to_storage(&s, ge);
memcpy(&pubkey->data[0], &s, 64);
}

int secp256k1_ec_pubkey_parse(const secp256k1_context* ctx, secp256k1_pubkey* pubkey, const unsigned char *input, size_t inputlen) {
Expand Down

0 comments on commit e53c2d9

Please sign in to comment.