Avoid modifying stack in sk_find.

Bug: 828680
Change-Id: Iae5d0a9bf938a67bfd69a720126ab431d79e43ec
Reviewed-on: https://boringssl-review.googlesource.com/27304
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
This commit is contained in:
Steven Valdez 2018-04-11 12:58:27 -04:00 committed by CQ bot account: commit-bot@chromium.org
parent c5154f7dbc
commit acddb8c134
15 changed files with 47 additions and 13 deletions

View File

@ -223,6 +223,7 @@ ASN1_STRING_TABLE *ASN1_STRING_TABLE_get(int nid)
return ttmp;
if (!stable)
return NULL;
sk_ASN1_STRING_TABLE_sort(stable);
found = sk_ASN1_STRING_TABLE_find(stable, &idx, &fnd);
if (!found)
return NULL;

View File

@ -223,7 +223,7 @@ void *sk_delete_ptr(_STACK *sk, void *p) {
return NULL;
}
int sk_find(_STACK *sk, size_t *out_index, void *p) {
int sk_find(const _STACK *sk, size_t *out_index, void *p) {
if (sk == NULL) {
return 0;
}
@ -245,7 +245,17 @@ int sk_find(_STACK *sk, size_t *out_index, void *p) {
return 0;
}
sk_sort(sk);
if (!sk_is_sorted(sk)) {
for (size_t i = 0; i < sk->num; i++) {
if (sk->comp((const void **)&p, (const void **)&sk->data[i]) == 0) {
if (out_index) {
*out_index = i;
}
return 1;
}
}
return 0;
}
// sk->comp is a function that takes pointers to pointers to elements, but
// qsort and bsearch take a comparison function that just takes pointers to

View File

@ -387,6 +387,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name,
*/
CRYPTO_MUTEX_lock_write(&xl->store_ctx->objs_lock);
tmp = NULL;
sk_X509_OBJECT_sort(xl->store_ctx->objs);
if (sk_X509_OBJECT_find(xl->store_ctx->objs, &idx, &stmp)) {
tmp = sk_X509_OBJECT_value(xl->store_ctx->objs, idx);
}
@ -404,6 +405,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name,
*/
if (!hent) {
htmp.hash = h;
sk_BY_DIR_HASH_sort(ent->hashes);
if (sk_BY_DIR_HASH_find(ent->hashes, &idx, &htmp))
hent = sk_BY_DIR_HASH_value(ent->hashes, idx);
}
@ -422,6 +424,7 @@ static int get_cert_by_subject(X509_LOOKUP *xl, int type, X509_NAME *name,
ok = 0;
goto finish;
}
sk_BY_DIR_HASH_sort(ent->hashes);
} else if (hent->suffix < k)
hent->suffix = k;

View File

@ -473,6 +473,7 @@ static int x509_object_idx_cnt(STACK_OF(X509_OBJECT) *h, int type,
}
size_t idx;
sk_X509_OBJECT_sort(h);
if (!sk_X509_OBJECT_find(h, &idx, &stmp))
return -1;
@ -604,6 +605,7 @@ X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h,
size_t idx, i;
X509_OBJECT *obj;
sk_X509_OBJECT_sort(h);
if (!sk_X509_OBJECT_find(h, &idx, x)) {
return NULL;
}

View File

@ -158,6 +158,7 @@ int X509_TRUST_get_by_id(int id)
tmp.trust = id;
if (!trtable)
return -1;
sk_X509_TRUST_sort(trtable);
if (!sk_X509_TRUST_find(trtable, &idx, &tmp)) {
return -1;
}

View File

@ -614,6 +614,7 @@ int X509_VERIFY_PARAM_add0_table(X509_VERIFY_PARAM *param)
} else {
size_t idx;
sk_X509_VERIFY_PARAM_sort(param_table);
if (sk_X509_VERIFY_PARAM_find(param_table, &idx, param)) {
ptmp = sk_X509_VERIFY_PARAM_value(param_table, idx);
X509_VERIFY_PARAM_free(ptmp);
@ -649,6 +650,7 @@ const X509_VERIFY_PARAM *X509_VERIFY_PARAM_lookup(const char *name)
pm.name = (char *)name;
if (param_table) {
size_t idx;
sk_X509_VERIFY_PARAM_sort(param_table);
if (sk_X509_VERIFY_PARAM_find(param_table, &idx, &pm))
return sk_X509_VERIFY_PARAM_value(param_table, idx);
}

View File

@ -93,6 +93,7 @@ static int policy_cache_create(X509 *x,
/*
* Duplicate policy OIDs are illegal: reject if matches found.
*/
sk_X509_POLICY_DATA_sort(cache->data);
if (OBJ_obj2nid(data->valid_policy) == NID_any_policy) {
if (cache->anyPolicy) {
ret = -1;
@ -262,6 +263,7 @@ X509_POLICY_DATA *policy_cache_find_data(const X509_POLICY_CACHE *cache,
X509_POLICY_DATA tmp;
tmp.valid_policy = (ASN1_OBJECT *)id;
sk_X509_POLICY_DATA_sort(cache->data);
if (!sk_X509_POLICY_DATA_find(cache->data, &idx, &tmp))
return NULL;
return sk_X509_POLICY_DATA_value(cache->data, idx);

View File

@ -83,6 +83,7 @@ X509_POLICY_NODE *tree_find_sk(STACK_OF(X509_POLICY_NODE) *nodes,
n.valid_policy = (ASN1_OBJECT *)id;
l.data = &n;
sk_X509_POLICY_NODE_sort(nodes);
if (!sk_X509_POLICY_NODE_find(nodes, &idx, &l))
return NULL;

View File

@ -543,9 +543,11 @@ static int tree_add_auth_node(STACK_OF(X509_POLICY_NODE) **pnodes,
*pnodes = policy_node_cmp_new();
if (!*pnodes)
return 0;
} else if (sk_X509_POLICY_NODE_find(*pnodes, NULL, pcy))
} else {
sk_X509_POLICY_NODE_sort(*pnodes);
if (sk_X509_POLICY_NODE_find(*pnodes, NULL, pcy))
return 1;
}
if (!sk_X509_POLICY_NODE_push(*pnodes, pcy))
return 0;

View File

@ -116,6 +116,7 @@ const X509V3_EXT_METHOD *X509V3_EXT_get_nid(int nid)
if (!ext_list)
return NULL;
sk_X509V3_EXT_METHOD_sort(ext_list);
if (!sk_X509V3_EXT_METHOD_find(ext_list, &idx, &tmp))
return NULL;
return sk_X509V3_EXT_METHOD_value(ext_list, idx);

View File

@ -205,6 +205,7 @@ int X509_PURPOSE_get_by_id(int purpose)
if (!xptable)
return -1;
sk_X509_PURPOSE_sort(xptable);
if (!sk_X509_PURPOSE_find(xptable, &idx, &tmp))
return -1;
return idx + X509_PURPOSE_COUNT;

View File

@ -650,6 +650,7 @@ static int append_ia5(STACK_OF(OPENSSL_STRING) **sk, ASN1_IA5STRING *email)
if (!*sk)
return 0;
/* Don't add duplicates */
sk_OPENSSL_STRING_sort(*sk);
if (sk_OPENSSL_STRING_find(*sk, NULL, (char *)email->data))
return 1;
emtmp = BUF_strdup((char *)email->data);

View File

@ -163,12 +163,17 @@ OPENSSL_EXPORT void *sk_delete(_STACK *sk, size_t where);
OPENSSL_EXPORT void *sk_delete_ptr(_STACK *sk, void *p);
// sk_find returns the first value in the stack equal to |p|. If a comparison
// function has been set on the stack, then equality is defined by it and the
// stack will be sorted if need be so that a binary search can be used.
// Otherwise pointer equality is used. If a matching element is found, its
// index is written to |*out_index| (if |out_index| is not NULL) and one is
// returned. Otherwise zero is returned.
OPENSSL_EXPORT int sk_find(_STACK *sk, size_t *out_index, void *p);
// function has been set on the stack, equality is defined by it, otherwise
// pointer equality is used. If the stack is sorted, then a binary search is
// used, otherwise a linear search is performed. If a matching element is found,
// its index is written to
// |*out_index| (if |out_index| is not NULL) and one is returned. Otherwise zero
// is returned.
//
// Note this differs from OpenSSL. The type signature is slightly different, and
// OpenSSL's sk_find will implicitly sort |sk| if it has a comparison function
// defined.
OPENSSL_EXPORT int sk_find(const _STACK *sk, size_t *out_index, void *p);
// sk_shift removes and returns the first element in the stack, or returns NULL
// if the stack is empty.
@ -302,8 +307,8 @@ struct StackTraits {};
} \
\
static inline OPENSSL_UNUSED int sk_##name##_find( \
STACK_OF(name) *sk, size_t *out_index, ptrtype p) { \
return sk_find((_STACK *)sk, out_index, (void *)p); \
const STACK_OF(name) *sk, size_t *out_index, ptrtype p) { \
return sk_find((const _STACK *)sk, out_index, (void *)p); \
} \
\
static inline OPENSSL_UNUSED ptrtype sk_##name##_shift(STACK_OF(name) *sk) { \

View File

@ -334,7 +334,7 @@ static const SSL_CIPHER *ssl3_choose_cipher(
SSL_HANDSHAKE *hs, const SSL_CLIENT_HELLO *client_hello,
const struct ssl_cipher_preference_list_st *server_pref) {
SSL *const ssl = hs->ssl;
STACK_OF(SSL_CIPHER) *prio, *allow;
const STACK_OF(SSL_CIPHER) *prio, *allow;
// in_group_flags will either be NULL, or will point to an array of bytes
// which indicate equal-preference groups in the |prio| stack. See the
// comment about |in_group_flags| in the |ssl_cipher_preference_list_st|

View File

@ -165,6 +165,7 @@ STACK_OF(X509_NAME) *SSL_load_client_CA_file(const char *file) {
}
// Check for duplicates.
sk_X509_NAME_sort(sk);
if (sk_X509_NAME_find(sk, NULL, xn)) {
continue;
}
@ -223,6 +224,7 @@ int SSL_add_file_cert_subjects_to_stack(STACK_OF(X509_NAME) *stack,
}
// Check for duplicates.
sk_X509_NAME_sort(stack);
if (sk_X509_NAME_find(stack, NULL, xn)) {
continue;
}