Avoid unions in X509_NAME logic.
Cast between T* and ASN1_VALUE* directly, rather than messing with
unions. This is necessary to avoid UB in C++ and is a strict aliasing
violation in C too. (While C does allow type-punning in unions, pointers
to union fields have weird rules. I suspect most of our unions should be
reworked.)
Change-Id: Ibe274a7df5d5826f8c5f335255d196e9b7587105
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42726
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/x509/x_name.c b/crypto/x509/x_name.c
index 7824100..bef9ec4 100644
--- a/crypto/x509/x_name.c
+++ b/crypto/x509/x_name.c
@@ -197,18 +197,8 @@
char opt, ASN1_TLC *ctx)
{
const unsigned char *p = *in, *q;
- union {
- STACK_OF(STACK_OF_X509_NAME_ENTRY) *s;
- ASN1_VALUE *a;
- } intname = {
- NULL
- };
- union {
- X509_NAME *x;
- ASN1_VALUE *a;
- } nm = {
- NULL
- };
+ STACK_OF(STACK_OF_X509_NAME_ENTRY) *intname = NULL;
+ X509_NAME *nm = NULL;
size_t i, j;
int ret;
STACK_OF(X509_NAME_ENTRY) *entries;
@@ -220,46 +210,48 @@
q = p;
/* Get internal representation of Name */
- ret = ASN1_item_ex_d2i(&intname.a,
+ ASN1_VALUE *intname_val = NULL;
+ ret = ASN1_item_ex_d2i(&intname_val,
&p, len, ASN1_ITEM_rptr(X509_NAME_INTERNAL),
tag, aclass, opt, ctx);
-
if (ret <= 0)
return ret;
+ intname = (STACK_OF(STACK_OF_X509_NAME_ENTRY) *)intname_val;
if (*val)
x509_name_ex_free(val, NULL);
- if (!x509_name_ex_new(&nm.a, NULL))
+ ASN1_VALUE *nm_val = NULL;
+ if (!x509_name_ex_new(&nm_val, NULL))
goto err;
+ nm = (X509_NAME *)nm_val;
/* We've decoded it: now cache encoding */
- if (!BUF_MEM_grow(nm.x->bytes, p - q))
+ if (!BUF_MEM_grow(nm->bytes, p - q))
goto err;
- OPENSSL_memcpy(nm.x->bytes->data, q, p - q);
+ OPENSSL_memcpy(nm->bytes->data, q, p - q);
/* Convert internal representation to X509_NAME structure */
- for (i = 0; i < sk_STACK_OF_X509_NAME_ENTRY_num(intname.s); i++) {
- entries = sk_STACK_OF_X509_NAME_ENTRY_value(intname.s, i);
+ for (i = 0; i < sk_STACK_OF_X509_NAME_ENTRY_num(intname); i++) {
+ 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;
- if (!sk_X509_NAME_ENTRY_push(nm.x->entries, entry))
+ if (!sk_X509_NAME_ENTRY_push(nm->entries, entry))
goto err;
(void)sk_X509_NAME_ENTRY_set(entries, j, NULL);
}
}
- ret = x509_name_canon(nm.x);
+ ret = x509_name_canon(nm);
if (!ret)
goto err;
- sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname.s,
+ sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname,
local_sk_X509_NAME_ENTRY_free);
- nm.x->modified = 0;
- *val = nm.a;
+ nm->modified = 0;
+ *val = (ASN1_VALUE *)nm;
*in = p;
return ret;
err:
- if (nm.x != NULL)
- X509_NAME_free(nm.x);
- sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname.s,
+ X509_NAME_free(nm);
+ sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname,
local_sk_X509_NAME_ENTRY_pop_free);
OPENSSL_PUT_ERROR(X509, ERR_R_ASN1_LIB);
return 0;
@@ -288,20 +280,15 @@
static int x509_name_encode(X509_NAME *a)
{
- union {
- STACK_OF(STACK_OF_X509_NAME_ENTRY) *s;
- ASN1_VALUE *a;
- } intname = {
- NULL
- };
int len;
unsigned char *p;
STACK_OF(X509_NAME_ENTRY) *entries = NULL;
X509_NAME_ENTRY *entry;
int set = -1;
size_t i;
- intname.s = sk_STACK_OF_X509_NAME_ENTRY_new_null();
- if (!intname.s)
+ STACK_OF(STACK_OF_X509_NAME_ENTRY) *intname =
+ sk_STACK_OF_X509_NAME_ENTRY_new_null();
+ if (!intname)
goto memerr;
for (i = 0; i < sk_X509_NAME_ENTRY_num(a->entries); i++) {
entry = sk_X509_NAME_ENTRY_value(a->entries, i);
@@ -309,7 +296,7 @@
entries = sk_X509_NAME_ENTRY_new_null();
if (!entries)
goto memerr;
- if (!sk_STACK_OF_X509_NAME_ENTRY_push(intname.s, entries)) {
+ if (!sk_STACK_OF_X509_NAME_ENTRY_push(intname, entries)) {
sk_X509_NAME_ENTRY_free(entries);
goto memerr;
}
@@ -318,19 +305,20 @@
if (!sk_X509_NAME_ENTRY_push(entries, entry))
goto memerr;
}
- len = ASN1_item_ex_i2d(&intname.a, NULL,
+ ASN1_VALUE *intname_val = (ASN1_VALUE *)intname;
+ len = ASN1_item_ex_i2d(&intname_val, NULL,
ASN1_ITEM_rptr(X509_NAME_INTERNAL), -1, -1);
if (!BUF_MEM_grow(a->bytes, len))
goto memerr;
p = (unsigned char *)a->bytes->data;
- ASN1_item_ex_i2d(&intname.a,
+ ASN1_item_ex_i2d(&intname_val,
&p, ASN1_ITEM_rptr(X509_NAME_INTERNAL), -1, -1);
- sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname.s,
+ sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname,
local_sk_X509_NAME_ENTRY_free);
a->modified = 0;
return len;
memerr:
- sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname.s,
+ sk_STACK_OF_X509_NAME_ENTRY_pop_free(intname,
local_sk_X509_NAME_ENTRY_free);
OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
return -1;
