Add int casts as needed around STACK_OF(T) sizes and indices We now ensure STACK_OF(T) sizes and indices fit in INT_MAX, so it's safe to cast to int. Bug: 516 Change-Id: I33dd1de6d60a852d510b9b5c3ac70e2eacbc8905 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60066 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/x509_att.c b/crypto/x509/x509_att.c index 23e92f2..062168e 100644 --- a/crypto/x509/x509_att.c +++ b/crypto/x509/x509_att.c
@@ -189,7 +189,7 @@ } int X509_ATTRIBUTE_count(const X509_ATTRIBUTE *attr) { - return sk_ASN1_TYPE_num(attr->set); + return (int)sk_ASN1_TYPE_num(attr->set); } ASN1_OBJECT *X509_ATTRIBUTE_get0_object(X509_ATTRIBUTE *attr) {
diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index cd4ed12..929afff 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c
@@ -423,9 +423,8 @@ } if (pnmatch != NULL) { - int tidx; *pnmatch = 1; - for (tidx = idx + 1; tidx < (int)sk_X509_OBJECT_num(h); tidx++) { + for (size_t tidx = idx + 1; tidx < sk_X509_OBJECT_num(h); tidx++) { const X509_OBJECT *tobj = sk_X509_OBJECT_value(h, tidx); if (x509_object_cmp(tobj, &stmp)) { break; @@ -434,7 +433,7 @@ } } - return idx; + return (int)idx; } int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, int type,
diff --git a/crypto/x509/x509_req.c b/crypto/x509/x509_req.c index 69eb63c..385d8c0 100644 --- a/crypto/x509/x509_req.c +++ b/crypto/x509/x509_req.c
@@ -161,7 +161,7 @@ } int X509_REQ_get_attr_count(const X509_REQ *req) { - return sk_X509_ATTRIBUTE_num(req->req_info->attributes); + return (int)sk_X509_ATTRIBUTE_num(req->req_info->attributes); } int X509_REQ_get_attr_by_NID(const X509_REQ *req, int nid, int lastpos) { @@ -181,7 +181,7 @@ if (lastpos < 0) { lastpos = 0; } - int n = sk_X509_ATTRIBUTE_num(req->req_info->attributes); + int n = (int)sk_X509_ATTRIBUTE_num(req->req_info->attributes); for (; lastpos < n; lastpos++) { const X509_ATTRIBUTE *attr = sk_X509_ATTRIBUTE_value(req->req_info->attributes, lastpos);
diff --git a/crypto/x509/x509_v3.c b/crypto/x509/x509_v3.c index 0d4ecfa..0f506c9 100644 --- a/crypto/x509/x509_v3.c +++ b/crypto/x509/x509_v3.c
@@ -69,7 +69,7 @@ if (x == NULL) { return 0; } - return (sk_X509_EXTENSION_num(x)); + return (int)sk_X509_EXTENSION_num(x); } int X509v3_get_ext_by_NID(const STACK_OF(X509_EXTENSION) *x, int nid, @@ -83,9 +83,6 @@ int X509v3_get_ext_by_OBJ(const STACK_OF(X509_EXTENSION) *sk, const ASN1_OBJECT *obj, int lastpos) { - int n; - X509_EXTENSION *ex; - if (sk == NULL) { return -1; } @@ -93,9 +90,9 @@ if (lastpos < 0) { lastpos = 0; } - n = sk_X509_EXTENSION_num(sk); + int n = (int)sk_X509_EXTENSION_num(sk); for (; lastpos < n; lastpos++) { - ex = sk_X509_EXTENSION_value(sk, lastpos); + const X509_EXTENSION *ex = sk_X509_EXTENSION_value(sk, lastpos); if (OBJ_cmp(ex->object, obj) == 0) { return lastpos; } @@ -115,7 +112,7 @@ } crit = !!crit; - int n = sk_X509_EXTENSION_num(sk); + int n = (int)sk_X509_EXTENSION_num(sk); for (; lastpos < n; lastpos++) { const X509_EXTENSION *ex = sk_X509_EXTENSION_value(sk, lastpos); if (X509_EXTENSION_get_critical(ex) == crit) { @@ -146,13 +143,12 @@ STACK_OF(X509_EXTENSION) *X509v3_add_ext(STACK_OF(X509_EXTENSION) **x, const X509_EXTENSION *ex, int loc) { X509_EXTENSION *new_ex = NULL; - int n; STACK_OF(X509_EXTENSION) *sk = NULL; int free_sk = 0; if (x == NULL) { OPENSSL_PUT_ERROR(X509, ERR_R_PASSED_NULL_PARAMETER); - goto err2; + goto err; } if (*x == NULL) { @@ -164,7 +160,7 @@ sk = *x; } - n = sk_X509_EXTENSION_num(sk); + int n = (int)sk_X509_EXTENSION_num(sk); if (loc > n) { loc = n; } else if (loc < 0) { @@ -172,7 +168,7 @@ } if ((new_ex = X509_EXTENSION_dup(ex)) == NULL) { - goto err2; + goto err; } if (!sk_X509_EXTENSION_insert(sk, new_ex, loc)) { goto err; @@ -181,8 +177,8 @@ *x = sk; } return sk; + err: -err2: X509_EXTENSION_free(new_ex); if (free_sk) { sk_X509_EXTENSION_free(sk);
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index ea9ff3c..a725d00 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c
@@ -217,7 +217,7 @@ goto end; } - num = sk_X509_num(ctx->chain); + num = (int)sk_X509_num(ctx->chain); x = sk_X509_value(ctx->chain, num - 1); depth = param->depth; @@ -284,7 +284,7 @@ do { // Examine last certificate in chain and see if it is self signed. - i = sk_X509_num(ctx->chain); + i = (int)sk_X509_num(ctx->chain); x = sk_X509_value(ctx->chain, i - 1); int is_self_signed; @@ -396,7 +396,7 @@ X509_free(xtmp); num--; } - ctx->last_untrusted = sk_X509_num(ctx->chain); + ctx->last_untrusted = (int)sk_X509_num(ctx->chain); retry = 1; break; } @@ -628,7 +628,7 @@ int i, j, rv; int has_name_constraints = 0; // Check name constraints for all certificates - for (i = sk_X509_num(ctx->chain) - 1; i >= 0; i--) { + for (i = (int)sk_X509_num(ctx->chain) - 1; i >= 0; i--) { X509 *x = sk_X509_value(ctx->chain, i); // Ignore self issued certs unless last in chain if (i && (x->ex_flags & EXFLAG_SI)) { @@ -638,7 +638,7 @@ // including trust anchor. Trust anchor not strictly speaking needed // but if it includes constraints it is to be assumed it expects them // to be obeyed. - for (j = sk_X509_num(ctx->chain) - 1; j > i; j--) { + for (j = (int)sk_X509_num(ctx->chain) - 1; j > i; j--) { NAME_CONSTRAINTS *nc = sk_X509_value(ctx->chain, j)->nc; if (nc) { has_name_constraints = 1; @@ -748,11 +748,10 @@ } static int check_trust(X509_STORE_CTX *ctx) { - size_t i; int ok; X509 *x = NULL; // Check all trusted certificates in chain - for (i = ctx->last_untrusted; i < sk_X509_num(ctx->chain); i++) { + for (size_t i = ctx->last_untrusted; i < sk_X509_num(ctx->chain); i++) { x = sk_X509_value(ctx->chain, i); ok = X509_check_trust(x, ctx->param->trust, 0); // If explicitly trusted return trusted @@ -762,7 +761,7 @@ // If explicitly rejected notify callback and reject if not // overridden. if (ok == X509_TRUST_REJECTED) { - ctx->error_depth = i; + ctx->error_depth = (int)i; ctx->current_cert = x; ctx->error = X509_V_ERR_CERT_REJECTED; ok = ctx->verify_cb(0, ctx); @@ -794,12 +793,12 @@ } static int check_revocation(X509_STORE_CTX *ctx) { - int i, last, ok; if (!(ctx->param->flags & X509_V_FLAG_CRL_CHECK)) { return 1; } + int last; if (ctx->param->flags & X509_V_FLAG_CRL_CHECK_ALL) { - last = sk_X509_num(ctx->chain) - 1; + last = (int)sk_X509_num(ctx->chain) - 1; } else { // If checking CRL paths this isn't the EE certificate if (ctx->parent) { @@ -807,9 +806,9 @@ } last = 0; } - for (i = 0; i <= last; i++) { + for (int i = 0; i <= last; i++) { ctx->error_depth = i; - ok = check_cert(ctx); + int ok = check_cert(ctx); if (!ok) { return ok; } @@ -1478,9 +1477,9 @@ static int check_crl(X509_STORE_CTX *ctx, X509_CRL *crl) { X509 *issuer = NULL; EVP_PKEY *ikey = NULL; - int ok = 0, chnum, cnum; - cnum = ctx->error_depth; - chnum = sk_X509_num(ctx->chain) - 1; + int ok = 0; + int cnum = ctx->error_depth; + int chnum = (int)sk_X509_num(ctx->chain) - 1; // if we have an alternative CRL issuer cert use that if (ctx->current_issuer) { issuer = ctx->current_issuer; @@ -1691,11 +1690,11 @@ } static int internal_verify(X509_STORE_CTX *ctx) { - int ok = 0, n; + int ok = 0; X509 *xs, *xi; EVP_PKEY *pkey = NULL; - n = sk_X509_num(ctx->chain); + int n = (int)sk_X509_num(ctx->chain); ctx->error_depth = n - 1; n--; xi = sk_X509_value(ctx->chain, n);
diff --git a/crypto/x509/x509name.c b/crypto/x509/x509name.c index 25f7b8b..eec2c8e 100644 --- a/crypto/x509/x509name.c +++ b/crypto/x509/x509name.c
@@ -99,7 +99,7 @@ if (name == NULL) { return 0; } - return (sk_X509_NAME_ENTRY_num(name->entries)); + return (int)sk_X509_NAME_ENTRY_num(name->entries); } int X509_NAME_get_index_by_NID(const X509_NAME *name, int nid, int lastpos) { @@ -109,26 +109,22 @@ if (obj == NULL) { return -2; } - return (X509_NAME_get_index_by_OBJ(name, obj, lastpos)); + return X509_NAME_get_index_by_OBJ(name, obj, lastpos); } // NOTE: you should be passsing -1, not 0 as lastpos int X509_NAME_get_index_by_OBJ(const X509_NAME *name, const ASN1_OBJECT *obj, int lastpos) { - int n; - X509_NAME_ENTRY *ne; - STACK_OF(X509_NAME_ENTRY) *sk; - if (name == NULL) { return -1; } if (lastpos < 0) { lastpos = -1; } - sk = name->entries; - n = sk_X509_NAME_ENTRY_num(sk); + const STACK_OF(X509_NAME_ENTRY) *sk = name->entries; + int n = (int)sk_X509_NAME_ENTRY_num(sk); for (lastpos++; lastpos < n; lastpos++) { - ne = sk_X509_NAME_ENTRY_value(sk, lastpos); + const X509_NAME_ENTRY *ne = sk_X509_NAME_ENTRY_value(sk, lastpos); if (OBJ_cmp(ne->object, obj) == 0) { return lastpos; } @@ -153,9 +149,9 @@ STACK_OF(X509_NAME_ENTRY) *sk = name->entries; X509_NAME_ENTRY *ret = sk_X509_NAME_ENTRY_delete(sk, loc); - int n = sk_X509_NAME_ENTRY_num(sk); + size_t n = sk_X509_NAME_ENTRY_num(sk); name->modified = 1; - if (loc == n) { + if ((size_t)loc == n) { return ret; } @@ -170,7 +166,7 @@ // If we removed a singleton RDN, update the RDN indices so they are // consecutive again. if (set_prev + 1 < set_next) { - for (int i = loc; i < n; i++) { + for (size_t i = loc; i < n; i++) { sk_X509_NAME_ENTRY_value(sk, i)->set--; } } @@ -221,14 +217,14 @@ int X509_NAME_add_entry(X509_NAME *name, const X509_NAME_ENTRY *entry, int loc, int set) { X509_NAME_ENTRY *new_name = NULL; - int n, i, inc; + int i, inc; STACK_OF(X509_NAME_ENTRY) *sk; if (name == NULL) { return 0; } sk = name->entries; - n = sk_X509_NAME_ENTRY_num(sk); + int n = (int)sk_X509_NAME_ENTRY_num(sk); if (loc > n) { loc = n; } else if (loc < 0) { @@ -266,7 +262,7 @@ goto err; } if (inc) { - n = sk_X509_NAME_ENTRY_num(sk); + n = (int)sk_X509_NAME_ENTRY_num(sk); for (i = loc + 1; i < n; i++) { sk_X509_NAME_ENTRY_value(sk, i)->set += 1; }
diff --git a/crypto/x509/x_name.c b/crypto/x509/x_name.c index a65c116..3063ce7 100644 --- a/crypto/x509/x_name.c +++ b/crypto/x509/x_name.c
@@ -230,7 +230,7 @@ entries = sk_STACK_OF_X509_NAME_ENTRY_value(intname, i); for (j = 0; j < sk_X509_NAME_ENTRY_num(entries); j++) { entry = sk_X509_NAME_ENTRY_value(entries, j); - entry->set = i; + entry->set = (int)i; if (!sk_X509_NAME_ENTRY_push(nm->entries, entry)) { goto err; }
diff --git a/crypto/x509v3/v3_lib.c b/crypto/x509v3/v3_lib.c index 61cbeea..d25b839 100644 --- a/crypto/x509v3/v3_lib.c +++ b/crypto/x509v3/v3_lib.c
@@ -195,7 +195,6 @@ void *X509V3_get_d2i(const STACK_OF(X509_EXTENSION) *extensions, int nid, int *out_critical, int *out_idx) { int lastpos; - size_t i; X509_EXTENSION *ex, *found_ex = NULL; if (!extensions) { if (out_idx) { @@ -214,13 +213,13 @@ if (lastpos < 0) { lastpos = 0; } - for (i = lastpos; i < sk_X509_EXTENSION_num(extensions); i++) { + for (size_t i = lastpos; i < sk_X509_EXTENSION_num(extensions); i++) { ex = sk_X509_EXTENSION_value(extensions, i); if (OBJ_obj2nid(ex->object) == nid) { if (out_idx) { // TODO(https://crbug.com/boringssl/379): Consistently reject // duplicate extensions. - *out_idx = i; + *out_idx = (int)i; found_ex = ex; break; } else if (found_ex) {