Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable -Wsign-Compare-check_bin/_crypto/_stuffer/_utils/ #3825

Merged
merged 16 commits into from
Feb 16, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bin/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ char *load_file_to_cstring(const char *path)
return NULL;
}

const long int pem_file_size = ftell(pem_file);
const ssize_t pem_file_size = ftell(pem_file);
if (pem_file_size < 0) {
fprintf(stderr, "Failed calling ftell: '%s'\n", strerror(errno));
fclose(pem_file);
Expand All @@ -158,7 +158,7 @@ char *load_file_to_cstring(const char *path)
return NULL;
}

if (fread(pem_out, sizeof(char), pem_file_size, pem_file) < pem_file_size) {
if (fread(pem_out, sizeof(char), pem_file_size, pem_file) < (size_t) pem_file_size) {
fprintf(stderr, "Failed reading file: '%s'\n", strerror(errno));
free(pem_out);
fclose(pem_file);
Expand Down
2 changes: 1 addition & 1 deletion crypto/s2n_cbc_cipher_3des.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ static int s2n_cbc_cipher_3des_encrypt(struct s2n_session_key *key, struct s2n_b
/* len is set by EVP_EncryptUpdate and checked post operation */
int len = 0;
POSIX_GUARD_OSSL(EVP_EncryptUpdate(key->evp_cipher_ctx, out->data, &len, in->data, in->size), S2N_ERR_ENCRYPT);
S2N_ERROR_IF(len != in->size, S2N_ERR_ENCRYPT);
POSIX_ENSURE((int64_t) len == (int64_t) in->size, S2N_ERR_ENCRYPT);

return 0;
}
Expand Down
2 changes: 1 addition & 1 deletion crypto/s2n_cbc_cipher_aes.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ static int s2n_cbc_cipher_aes_encrypt(struct s2n_session_key *key, struct s2n_bl
/* len is set by EVP_EncryptUpdate and checked post operation */
int len = 0;
POSIX_GUARD_OSSL(EVP_EncryptUpdate(key->evp_cipher_ctx, out->data, &len, in->data, in->size), S2N_ERR_ENCRYPT);
S2N_ERROR_IF(len != in->size, S2N_ERR_ENCRYPT);
POSIX_ENSURE((int64_t) len == (int64_t) in->size, S2N_ERR_ENCRYPT);

return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions crypto/s2n_certificate.c
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ static int s2n_utf8_string_from_extension_data(const uint8_t *extension_data, ui

int len = ASN1_STRING_length(asn1_str);
if (out_data != NULL) {
POSIX_ENSURE(*out_len >= len, S2N_ERR_INSUFFICIENT_MEM_SIZE);
POSIX_ENSURE((int64_t) *out_len >= (int64_t) len, S2N_ERR_INSUFFICIENT_MEM_SIZE);
/* ASN1_STRING_data() returns an internal pointer to the data.
* Since this is an internal pointer it should not be freed or modified in any way.
* Ref: https://www.openssl.org/docs/man1.0.2/man3/ASN1_STRING_data.html.
Expand Down Expand Up @@ -776,7 +776,7 @@ static int s2n_parse_x509_extension(struct s2n_cert *cert, const uint8_t *oid,
* X509_get_ext_count returns the number of extensions in the x509 certificate.
* Ref: https://www.openssl.org/docs/man1.1.0/man3/X509_get_ext_count.html.
*/
int ext_count = X509_get_ext_count(x509_cert);
size_t ext_count = X509_get_ext_count(x509_cert);
POSIX_ENSURE_GT(ext_count, 0);

/* OBJ_txt2obj() converts the input text string into an ASN1_OBJECT structure.
Expand Down
2 changes: 1 addition & 1 deletion crypto/s2n_composite_cipher_aes_sha.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ static int s2n_composite_cipher_aes_sha_encrypt(struct s2n_session_key *key, str
int len = 0;
POSIX_GUARD_OSSL(EVP_EncryptUpdate(key->evp_cipher_ctx, out->data, &len, in->data, in->size), S2N_ERR_ENCRYPT);

S2N_ERROR_IF(len != in->size, S2N_ERR_ENCRYPT);
POSIX_ENSURE((int64_t) len == (int64_t) in->size, S2N_ERR_ENCRYPT);

return 0;
}
Expand Down
8 changes: 4 additions & 4 deletions crypto/s2n_drbg.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ static bool ignore_prediction_resistance_for_testing = false;
acceptable in DRBG */
S2N_RESULT s2n_increment_drbg_counter(struct s2n_blob *counter)
{
for (uint32_t i = counter->size; i > 0; i--) {
for (uint32_t i = (uint32_t) counter->size; i > 0; i--) {
counter->data[i - 1] += 1;
if (counter->data[i - 1]) {
break;
Expand Down Expand Up @@ -63,10 +63,10 @@ static S2N_RESULT s2n_drbg_bits(struct s2n_drbg *drbg, struct s2n_blob *out)

struct s2n_blob value = { 0 };
RESULT_GUARD_POSIX(s2n_blob_init(&value, drbg->v, sizeof(drbg->v)));
int block_aligned_size = out->size - (out->size % S2N_DRBG_BLOCK_SIZE);
uint32_t block_aligned_size = out->size - (out->size % S2N_DRBG_BLOCK_SIZE);

/* Per NIST SP800-90A 10.2.1.2: */
for (int i = 0; i < block_aligned_size; i += S2N_DRBG_BLOCK_SIZE) {
for (size_t i = 0; i < block_aligned_size; i += S2N_DRBG_BLOCK_SIZE) {
RESULT_GUARD(s2n_increment_drbg_counter(&value));
RESULT_GUARD(s2n_drbg_block_encrypt(drbg->ctx, drbg->v, out->data + i));
drbg->bytes_used += S2N_DRBG_BLOCK_SIZE;
Expand Down Expand Up @@ -94,7 +94,7 @@ static S2N_RESULT s2n_drbg_update(struct s2n_drbg *drbg, struct s2n_blob *provid

RESULT_STACK_BLOB(temp_blob, s2n_drbg_seed_size(drgb), S2N_DRBG_MAX_SEED_SIZE);

RESULT_ENSURE_EQ(provided_data->size, s2n_drbg_seed_size(drbg));
RESULT_ENSURE_EQ(provided_data->size, (uint32_t) s2n_drbg_seed_size(drbg));

RESULT_GUARD(s2n_drbg_bits(drbg, &temp_blob));

Expand Down
4 changes: 2 additions & 2 deletions crypto/s2n_hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -388,14 +388,14 @@ static int s2n_evp_hash_digest(struct s2n_hash_state *state, void *out, uint32_t
unsigned int md5_secondary_digest_size = digest_size - sha1_primary_digest_size;

POSIX_ENSURE(EVP_MD_CTX_size(state->digest.high_level.evp.ctx) <= sha1_digest_size, S2N_ERR_HASH_DIGEST_FAILED);
POSIX_ENSURE(EVP_MD_CTX_size(state->digest.high_level.evp_md5_secondary.ctx) <= md5_secondary_digest_size, S2N_ERR_HASH_DIGEST_FAILED);
POSIX_ENSURE((size_t) EVP_MD_CTX_size(state->digest.high_level.evp_md5_secondary.ctx) <= md5_secondary_digest_size, S2N_ERR_HASH_DIGEST_FAILED);

POSIX_GUARD_OSSL(EVP_DigestFinal_ex(state->digest.high_level.evp.ctx, ((uint8_t *) out) + MD5_DIGEST_LENGTH, &sha1_primary_digest_size), S2N_ERR_HASH_DIGEST_FAILED);
POSIX_GUARD_OSSL(EVP_DigestFinal_ex(state->digest.high_level.evp_md5_secondary.ctx, out, &md5_secondary_digest_size), S2N_ERR_HASH_DIGEST_FAILED);
return S2N_SUCCESS;
}

POSIX_ENSURE(EVP_MD_CTX_size(state->digest.high_level.evp.ctx) <= digest_size, S2N_ERR_HASH_DIGEST_FAILED);
POSIX_ENSURE((size_t) EVP_MD_CTX_size(state->digest.high_level.evp.ctx) <= digest_size, S2N_ERR_HASH_DIGEST_FAILED);
POSIX_GUARD_OSSL(EVP_DigestFinal_ex(state->digest.high_level.evp.ctx, out, &digest_size), S2N_ERR_HASH_DIGEST_FAILED);
return S2N_SUCCESS;
}
Expand Down
4 changes: 2 additions & 2 deletions crypto/s2n_rsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ static int s2n_rsa_encrypt(const struct s2n_pkey *pub, struct s2n_blob *in, stru
/* Safety: RSA_public_encrypt does not mutate the key */
int r = RSA_public_encrypt(in->size, (unsigned char *) in->data, (unsigned char *) out->data,
s2n_unsafe_rsa_get_non_const(pub_key), RSA_PKCS1_PADDING);
S2N_ERROR_IF(r != out->size, S2N_ERR_SIZE_MISMATCH);
POSIX_ENSURE((int64_t) r == (int64_t) out->size, S2N_ERR_SIZE_MISMATCH);

return 0;
}
Expand All @@ -143,7 +143,7 @@ static int s2n_rsa_decrypt(const struct s2n_pkey *priv, struct s2n_blob *in, str
/* Safety: RSA_private_decrypt does not mutate the key */
int r = RSA_private_decrypt(in->size, (unsigned char *) in->data, intermediate,
s2n_unsafe_rsa_get_non_const(priv_key), RSA_NO_PADDING);
S2N_ERROR_IF(r != expected_size, S2N_ERR_SIZE_MISMATCH);
POSIX_ENSURE((int64_t) r == expected_size, S2N_ERR_SIZE_MISMATCH);

s2n_constant_time_pkcs1_unpad_or_dont(out->data, intermediate, r, out->size);

Expand Down
4 changes: 2 additions & 2 deletions crypto/s2n_stream_cipher_rc4.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ static int s2n_stream_cipher_rc4_encrypt(struct s2n_session_key *key, struct s2n
int len = 0;
POSIX_GUARD_OSSL(EVP_EncryptUpdate(key->evp_cipher_ctx, out->data, &len, in->data, in->size), S2N_ERR_ENCRYPT);

S2N_ERROR_IF(len != in->size, S2N_ERR_ENCRYPT);
POSIX_ENSURE((int64_t) len == (int64_t) in->size, S2N_ERR_DECRYPT);

return 0;
}
Expand All @@ -56,7 +56,7 @@ static int s2n_stream_cipher_rc4_decrypt(struct s2n_session_key *key, struct s2n
int len = 0;
POSIX_GUARD_OSSL(EVP_DecryptUpdate(key->evp_cipher_ctx, out->data, &len, in->data, in->size), S2N_ERR_DECRYPT);

S2N_ERROR_IF(len != in->size, S2N_ERR_DECRYPT);
POSIX_ENSURE((int64_t) len == (int64_t) in->size, S2N_ERR_DECRYPT);

return 0;
}
Expand Down
3 changes: 2 additions & 1 deletion stuffer/s2n_stuffer_network_order.c
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ static int length_matches_value_check(uint32_t value, uint8_t length)

if (length < sizeof(uint32_t)) {
/* Value should be less than the maximum for its length */
POSIX_ENSURE(value < (0x01 << (length * 8)), S2N_ERR_SIZE_MISMATCH);
const uint32_t size_max = 1 << (length * 8);
POSIX_ENSURE(value < size_max, S2N_ERR_SIZE_MISMATCH);
}

return S2N_SUCCESS;
Expand Down
2 changes: 1 addition & 1 deletion stuffer/s2n_stuffer_text.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ int s2n_stuffer_skip_read_until(struct s2n_stuffer *stuffer, const char *target)
{
POSIX_PRECONDITION(s2n_stuffer_validate(stuffer));
POSIX_ENSURE_REF(target);
const int len = strlen(target);
const uint32_t len = strlen(target);
if (len == 0) {
return S2N_SUCCESS;
}
Expand Down
4 changes: 2 additions & 2 deletions utils/s2n_map.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ static S2N_RESULT s2n_map_embiggen(struct s2n_map *map, uint32_t capacity)
tmp.table = (void *) mem.data;
tmp.immutable = 0;

for (uint32_t i = 0; i < map->capacity; i++) {
for (size_t i = 0; i < map->capacity; i++) {
if (map->table[i].key.size) {
RESULT_GUARD(s2n_map_add(&tmp, &map->table[i].key, &map->table[i].value));
RESULT_GUARD_POSIX(s2n_free(&map->table[i].key));
Expand Down Expand Up @@ -231,7 +231,7 @@ S2N_RESULT s2n_map_free(struct s2n_map *map)
/* Free the keys and values */
/* cppcheck has a false positive warning for checking the pointer here */
/* cppcheck-suppress nullPointerRedundantCheck */
for (uint32_t i = 0; i < map->capacity; i++) {
for (size_t i = 0; i < map->capacity; i++) {
if (map->table[i].key.size) {
RESULT_GUARD_POSIX(s2n_free(&map->table[i].key));
RESULT_GUARD_POSIX(s2n_free(&map->table[i].value));
Expand Down
4 changes: 2 additions & 2 deletions utils/s2n_random.c
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ static int s2n_rand_rdrand_impl(void *data, uint32_t size)
#if defined(__x86_64__) || defined(__i386__)
struct s2n_blob out = { 0 };
POSIX_GUARD(s2n_blob_init(&out, data, size));
int space_remaining = 0;
size_t space_remaining = 0;
struct s2n_stuffer stuffer = { 0 };
union {
uint64_t u64;
Expand Down Expand Up @@ -583,7 +583,7 @@ static int s2n_rand_rdrand_impl(void *data, uint32_t size)

POSIX_ENSURE(success, S2N_ERR_RDRAND_FAILED);

int data_to_fill = MIN(sizeof(output), space_remaining);
size_t data_to_fill = MIN(sizeof(output), space_remaining);

POSIX_GUARD(s2n_stuffer_write_bytes(&stuffer, output.u8, data_to_fill));
}
Expand Down
6 changes: 3 additions & 3 deletions utils/s2n_safety.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ bool s2n_constant_time_equals(const uint8_t *a, const uint8_t *b, const uint32_t
uint8_t xor = !((a_inc == 1) & (b_inc == 1));

/* iterate over each byte in the slices */
for (uint32_t i = 0; i < len; i++) {
for (size_t i = 0; i < len; i++) {
/* Invariants must hold for each execution of the loop
* and at loop exit, hence the <= */
S2N_INVARIANT(i <= len);
Expand Down Expand Up @@ -99,7 +99,7 @@ int s2n_constant_time_copy_or_dont(uint8_t *dest, const uint8_t *src, uint32_t l
/* dont = 0 : mask = 0xff */
/* dont > 0 : mask = 0x00 */

for (uint32_t i = 0; i < len; i++) {
for (size_t i = 0; i < len; i++) {
uint8_t old = dest[i];
uint8_t diff = (old ^ src[i]) & mask;
dest[i] = old ^ diff;
Expand Down Expand Up @@ -140,7 +140,7 @@ int s2n_constant_time_pkcs1_unpad_or_dont(uint8_t *dst, const uint8_t *src, uint
dont_copy |= src[1] ^ 0x02;
dont_copy |= *(start_of_data - 1) ^ 0x00;

for (uint32_t i = 2; i < srclen - expectlen - 1; i++) {
for (size_t i = 2; i < srclen - expectlen - 1; i++) {
/* Note! We avoid using logical NOT (!) here; while in practice
* many compilers will use constant-time sequences for this operator,
* at least on x86 (e.g. cmp -> setcc, or vectorized pcmpeq), this is
Expand Down