Require that sizeof(secp256k1_ge_storage) == 64

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.
This commit is contained in:
Tim Ruffing 2024-01-08 15:51:15 +01:00
parent d0ba2abbff
commit e53c2d9ffc

View File

@ -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) { static int secp256k1_pubkey_load(const secp256k1_context* ctx, secp256k1_ge* ge, const secp256k1_pubkey* pubkey) {
if (sizeof(secp256k1_ge_storage) == 64) { secp256k1_ge_storage s;
/* When the secp256k1_ge_storage type is exactly 64 byte, use its
* representation inside secp256k1_pubkey, as conversion is very fast. /* We require that the secp256k1_ge_storage type is exactly 64 bytes.
* Note that secp256k1_pubkey_save must use the same representation. */ * This is formally not guaranteed by the C standard, but should hold on any
secp256k1_ge_storage s; * sane compiler in the real world. */
memcpy(&s, &pubkey->data[0], sizeof(s)); STATIC_ASSERT(sizeof(secp256k1_ge_storage) == 64);
secp256k1_ge_from_storage(ge, &s); memcpy(&s, &pubkey->data[0], 64);
} else { secp256k1_ge_from_storage(ge, &s);
/* 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);
}
ARG_CHECK(!secp256k1_fe_is_zero(&ge->x)); ARG_CHECK(!secp256k1_fe_is_zero(&ge->x));
return 1; return 1;
} }
static void secp256k1_pubkey_save(secp256k1_pubkey* pubkey, secp256k1_ge* ge) { static void secp256k1_pubkey_save(secp256k1_pubkey* pubkey, secp256k1_ge* ge) {
if (sizeof(secp256k1_ge_storage) == 64) { secp256k1_ge_storage s;
secp256k1_ge_storage s;
secp256k1_ge_to_storage(&s, ge); STATIC_ASSERT(sizeof(secp256k1_ge_storage) == 64);
memcpy(&pubkey->data[0], &s, sizeof(s)); VERIFY_CHECK(!secp256k1_ge_is_infinity(ge));
} else { secp256k1_ge_to_storage(&s, ge);
VERIFY_CHECK(!secp256k1_ge_is_infinity(ge)); memcpy(&pubkey->data[0], &s, 64);
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);
}
} }
int secp256k1_ec_pubkey_parse(const secp256k1_context* ctx, secp256k1_pubkey* pubkey, const unsigned char *input, size_t inputlen) { int secp256k1_ec_pubkey_parse(const secp256k1_context* ctx, secp256k1_pubkey* pubkey, const unsigned char *input, size_t inputlen) {