Enable -Wsign-Compare-check_bin/_crypto/_stuffer/_utils/ (aws#3825)

bin/common.c

@@ -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);
@@ -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);

crypto/s2n_cbc_cipher_3des.c

@@ -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;
 }

crypto/s2n_cbc_cipher_aes.c

@@ -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;
 }

crypto/s2n_certificate.c

@@ -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.
@@ -776,8 +776,9 @@ 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);
-    POSIX_ENSURE_GT(ext_count, 0);
+    int ext_count_value = X509_get_ext_count(x509_cert);
+    POSIX_ENSURE_GT(ext_count_value, 0);
+    size_t ext_count = (size_t) ext_count_value;
 
     /* OBJ_txt2obj() converts the input text string into an ASN1_OBJECT structure.
      * If no_name is 0 then long names and short names will be interpreted as well as numerical forms.

crypto/s2n_composite_cipher_aes_sha.c

@@ -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;
 }

crypto/s2n_drbg.c

@@ -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;
@@ -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;
@@ -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));
 

crypto/s2n_hash.c

@@ -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;
 }

crypto/s2n_rsa.c

@@ -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;
 }
@@ -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 == (int64_t) expected_size, S2N_ERR_SIZE_MISMATCH);
 
     s2n_constant_time_pkcs1_unpad_or_dont(out->data, intermediate, r, out->size);
 

crypto/s2n_stream_cipher_rc4.c

@@ -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;
 }
@@ -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;
 }

stuffer/s2n_stuffer_network_order.c

@@ -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;

stuffer/s2n_stuffer_text.c

@@ -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;
     }

utils/s2n_map.c

@@ -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));
@@ -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));

utils/s2n_random.c

@@ -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;
@@ -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));
     }

utils/s2n_safety.c

@@ -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);
@@ -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;
@@ -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