Skip to content

Commit

Permalink
Stop raising ERR_R_MALLOC_FAILURE in most places
Browse files Browse the repository at this point in the history
Since OPENSSL_malloc() and friends report ERR_R_MALLOC_FAILURE, and
at least handle the file name and line number they are called from,
there's no need to report ERR_R_MALLOC_FAILURE where they are called
directly, or when SSLfatal() and RLAYERfatal() is used, the reason
`ERR_R_MALLOC_FAILURE` is changed to `ERR_R_CRYPTO_LIB`.

There were a number of places where `ERR_R_MALLOC_FAILURE` was reported
even though it was a function from a different sub-system that was
called.  Those places are changed to report ERR_R_{lib}_LIB, where
{lib} is the name of that sub-system.
Some of them are tricky to get right, as we have a lot of functions
that belong in the ASN1 sub-system, and all the `sk_` calls or from
the CRYPTO sub-system.

Some extra adaptation was necessary where there were custom OPENSSL_malloc()
wrappers, and some bugs are fixed alongside these changes.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from #19301)
  • Loading branch information
levitte committed Oct 5, 2022
1 parent 9167a47 commit e077455
Show file tree
Hide file tree
Showing 380 changed files with 2,220 additions and 2,778 deletions.
10 changes: 4 additions & 6 deletions crypto/asn1/a_bitstr.c
Expand Up @@ -82,7 +82,7 @@ ASN1_BIT_STRING *ossl_c2i_ASN1_BIT_STRING(ASN1_BIT_STRING **a,
ASN1_BIT_STRING *ret = NULL;
const unsigned char *p;
unsigned char *s;
int i;
int i = 0;

if (len < 1) {
i = ASN1_R_STRING_TOO_SHORT;
Expand Down Expand Up @@ -115,7 +115,6 @@ ASN1_BIT_STRING *ossl_c2i_ASN1_BIT_STRING(ASN1_BIT_STRING **a,
if (len-- > 1) { /* using one because of the bits left byte */
s = OPENSSL_malloc((int)len);
if (s == NULL) {
i = ERR_R_MALLOC_FAILURE;
goto err;
}
memcpy(s, p, (int)len);
Expand All @@ -131,7 +130,8 @@ ASN1_BIT_STRING *ossl_c2i_ASN1_BIT_STRING(ASN1_BIT_STRING **a,
*pp = p;
return ret;
err:
ERR_raise(ERR_LIB_ASN1, i);
if (i != 0)
ERR_raise(ERR_LIB_ASN1, i);
if ((a == NULL) || (*a != ret))
ASN1_BIT_STRING_free(ret);
return NULL;
Expand Down Expand Up @@ -160,10 +160,8 @@ int ASN1_BIT_STRING_set_bit(ASN1_BIT_STRING *a, int n, int value)
if (!value)
return 1; /* Don't need to set */
c = OPENSSL_clear_realloc(a->data, a->length, w + 1);
if (c == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if (c == NULL)
return 0;
}
if (w + 1 - a->length > 0)
memset(c + a->length, 0, w + 1 - a->length);
a->data = c;
Expand Down
6 changes: 3 additions & 3 deletions crypto/asn1/a_d2i_fp.c
Expand Up @@ -123,7 +123,7 @@ int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)

b = BUF_MEM_new();
if (b == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_BUF_LIB);
return -1;
}

Expand All @@ -134,7 +134,7 @@ int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
want -= diff;

if (len + want < len || !BUF_MEM_grow_clean(b, len + want)) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_BUF_LIB);
goto err;
}
i = BIO_read(in, &(b->data[len]), want);
Expand Down Expand Up @@ -206,7 +206,7 @@ int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
size_t chunk = want > chunk_max ? chunk_max : want;

if (!BUF_MEM_grow_clean(b, len + chunk)) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_BUF_LIB);
goto err;
}
want -= chunk;
Expand Down
4 changes: 1 addition & 3 deletions crypto/asn1/a_digest.c
Expand Up @@ -36,10 +36,8 @@ int ASN1_digest(i2d_of_void *i2d, const EVP_MD *type, char *data,
ERR_raise(ERR_LIB_ASN1, ERR_R_INTERNAL_ERROR);
return 0;
}
if ((str = OPENSSL_malloc(inl)) == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if ((str = OPENSSL_malloc(inl)) == NULL)
return 0;
}
p = str;
i2d(data, &p);

Expand Down
6 changes: 2 additions & 4 deletions crypto/asn1/a_dup.c
Expand Up @@ -28,10 +28,8 @@ void *ASN1_dup(i2d_of_void *i2d, d2i_of_void *d2i, const void *x)
return NULL;

b = OPENSSL_malloc(i + 10);
if (b == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if (b == NULL)
return NULL;
}
p = b;
i = i2d(x, &p);
p2 = b;
Expand Down Expand Up @@ -78,7 +76,7 @@ void *ASN1_item_dup(const ASN1_ITEM *it, const void *x)

i = ASN1_item_i2d(x, &b, it);
if (b == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_ASN1_LIB);
return NULL;
}
p = b;
Expand Down
6 changes: 2 additions & 4 deletions crypto/asn1/a_i2d_fp.c
Expand Up @@ -42,10 +42,8 @@ int ASN1_i2d_bio(i2d_of_void *i2d, BIO *out, const void *x)
return 0;

b = OPENSSL_malloc(n);
if (b == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if (b == NULL)
return 0;
}

p = (unsigned char *)b;
i2d(x, &p);
Expand Down Expand Up @@ -91,7 +89,7 @@ int ASN1_item_i2d_bio(const ASN1_ITEM *it, BIO *out, const void *x)

n = ASN1_item_i2d(x, &b, it);
if (b == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_ASN1_LIB);
return 0;
}

Expand Down
16 changes: 8 additions & 8 deletions crypto/asn1/a_int.c
Expand Up @@ -303,8 +303,10 @@ ASN1_INTEGER *ossl_c2i_ASN1_INTEGER(ASN1_INTEGER **a, const unsigned char **pp,
} else
ret = *a;

if (ASN1_STRING_set(ret, NULL, r) == 0)
if (ASN1_STRING_set(ret, NULL, r) == 0) {
ERR_raise(ERR_LIB_ASN1, ERR_R_ASN1_LIB);
goto err;
}

c2i_ibuf(ret->data, &neg, *pp, len);

Expand All @@ -318,7 +320,6 @@ ASN1_INTEGER *ossl_c2i_ASN1_INTEGER(ASN1_INTEGER **a, const unsigned char **pp,
(*a) = ret;
return ret;
err:
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if (a == NULL || *a != ret)
ASN1_INTEGER_free(ret);
return NULL;
Expand Down Expand Up @@ -400,7 +401,7 @@ ASN1_INTEGER *d2i_ASN1_UINTEGER(ASN1_INTEGER **a, const unsigned char **pp,
unsigned char *s;
long len = 0;
int inf, tag, xclass;
int i;
int i = 0;

if ((a == NULL) || ((*a) == NULL)) {
if ((ret = ASN1_INTEGER_new()) == NULL)
Expand Down Expand Up @@ -430,10 +431,8 @@ ASN1_INTEGER *d2i_ASN1_UINTEGER(ASN1_INTEGER **a, const unsigned char **pp,
* a missing NULL parameter.
*/
s = OPENSSL_malloc((int)len + 1);
if (s == NULL) {
i = ERR_R_MALLOC_FAILURE;
if (s == NULL)
goto err;
}
ret->type = V_ASN1_INTEGER;
if (len) {
if ((*p == 0) && (len != 1)) {
Expand All @@ -450,7 +449,8 @@ ASN1_INTEGER *d2i_ASN1_UINTEGER(ASN1_INTEGER **a, const unsigned char **pp,
*pp = p;
return ret;
err:
ERR_raise(ERR_LIB_ASN1, i);
if (i != 0)
ERR_raise(ERR_LIB_ASN1, i);
if ((a == NULL) || (*a != ret))
ASN1_INTEGER_free(ret);
return NULL;
Expand Down Expand Up @@ -483,7 +483,7 @@ static ASN1_STRING *bn_to_asn1_string(const BIGNUM *bn, ASN1_STRING *ai,
len = 1;

if (ASN1_STRING_set(ret, NULL, len) == 0) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_ASN1_LIB);
goto err;
}

Expand Down
5 changes: 2 additions & 3 deletions crypto/asn1/a_mbstr.c
Expand Up @@ -145,15 +145,15 @@ int ASN1_mbstring_ncopy(ASN1_STRING **out, const unsigned char *in, int len,
free_out = 1;
dest = ASN1_STRING_type_new(str_type);
if (dest == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_ASN1_LIB);
return -1;
}
*out = dest;
}
/* If both the same type just copy across */
if (inform == outform) {
if (!ASN1_STRING_set(dest, in, len)) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_ASN1_LIB);
return -1;
}
return str_type;
Expand Down Expand Up @@ -185,7 +185,6 @@ int ASN1_mbstring_ncopy(ASN1_STRING **out, const unsigned char *in, int len,
if ((p = OPENSSL_malloc(outlen + 1)) == NULL) {
if (free_out)
ASN1_STRING_free(dest);
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
return -1;
}
dest->length = outlen;
Expand Down
20 changes: 5 additions & 15 deletions crypto/asn1/a_object.c
Expand Up @@ -31,10 +31,8 @@ int i2d_ASN1_OBJECT(const ASN1_OBJECT *a, unsigned char **pp)
return objsize;

if (*pp == NULL) {
if ((p = allocated = OPENSSL_malloc(objsize)) == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if ((p = allocated = OPENSSL_malloc(objsize)) == NULL)
return 0;
}
} else {
p = *pp;
}
Expand Down Expand Up @@ -135,10 +133,8 @@ int a2d_ASN1_OBJECT(unsigned char *out, int olen, const char *buf, int num)
OPENSSL_free(tmp);
tmpsize = blsize + 32;
tmp = OPENSSL_malloc(tmpsize);
if (tmp == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if (tmp == NULL)
goto err;
}
}
while (blsize--) {
BN_ULONG t = BN_div_word(bl, 0x80L);
Expand Down Expand Up @@ -196,10 +192,8 @@ int i2a_ASN1_OBJECT(BIO *bp, const ASN1_OBJECT *a)
ERR_raise(ERR_LIB_ASN1, ASN1_R_LENGTH_TOO_LONG);
return -1;
}
if ((p = OPENSSL_malloc(i + 1)) == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if ((p = OPENSSL_malloc(i + 1)) == NULL)
return -1;
}
i2t_ASN1_OBJECT(p, i + 1, a);
}
if (i <= 0) {
Expand Down Expand Up @@ -308,10 +302,8 @@ ASN1_OBJECT *ossl_c2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp,
ret->length = 0;
OPENSSL_free(data);
data = OPENSSL_malloc(length);
if (data == NULL) {
i = ERR_R_MALLOC_FAILURE;
if (data == NULL)
goto err;
}
ret->flags |= ASN1_OBJECT_FLAG_DYNAMIC_DATA;
}
memcpy(data, p, length);
Expand Down Expand Up @@ -345,10 +337,8 @@ ASN1_OBJECT *ASN1_OBJECT_new(void)
ASN1_OBJECT *ret;

ret = OPENSSL_zalloc(sizeof(*ret));
if (ret == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if (ret == NULL)
return NULL;
}
ret->flags = ASN1_OBJECT_FLAG_DYNAMIC;
return ret;
}
Expand Down
6 changes: 2 additions & 4 deletions crypto/asn1/a_sign.c
Expand Up @@ -35,7 +35,7 @@ int ASN1_sign(i2d_of_void *i2d, X509_ALGOR *algor1, X509_ALGOR *algor2,
X509_ALGOR *a;

if (ctx == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_EVP_LIB);
goto err;
}
for (i = 0; i < 2; i++) {
Expand Down Expand Up @@ -82,7 +82,6 @@ int ASN1_sign(i2d_of_void *i2d, X509_ALGOR *algor1, X509_ALGOR *algor2,
buf_out = OPENSSL_malloc(outll);
if (buf_in == NULL || buf_out == NULL) {
outl = 0;
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
goto err;
}
p = buf_in;
Expand Down Expand Up @@ -130,7 +129,7 @@ int ASN1_item_sign_ex(const ASN1_ITEM *it, X509_ALGOR *algor1,
EVP_MD_CTX *ctx = evp_md_ctx_new_ex(pkey, id, libctx, propq);

if (ctx == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_EVP_LIB);
return 0;
}
/* We can use the non _ex variant here since the pkey is already setup */
Expand Down Expand Up @@ -270,7 +269,6 @@ int ASN1_item_sign_ctx(const ASN1_ITEM *it, X509_ALGOR *algor1,
buf_out = OPENSSL_malloc(outll);
if (buf_in == NULL || buf_out == NULL) {
outl = 0;
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
goto err;
}

Expand Down
4 changes: 1 addition & 3 deletions crypto/asn1/a_strex.c
Expand Up @@ -282,10 +282,8 @@ static int do_dump(unsigned long lflags, char_io *io_ch, void *arg,
der_len = i2d_ASN1_TYPE(&t, NULL);
if (der_len <= 0)
return -1;
if ((der_buf = OPENSSL_malloc(der_len)) == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if ((der_buf = OPENSSL_malloc(der_len)) == NULL)
return -1;
}
p = der_buf;
i2d_ASN1_TYPE(&t, &p);
outlen = do_hex_dump(io_ch, arg, der_buf, der_len);
Expand Down
6 changes: 2 additions & 4 deletions crypto/asn1/a_strnid.c
Expand Up @@ -159,10 +159,8 @@ static ASN1_STRING_TABLE *stable_get(int nid)
tmp = ASN1_STRING_TABLE_get(nid);
if (tmp != NULL && tmp->flags & STABLE_FLAGS_MALLOC)
return tmp;
if ((rv = OPENSSL_zalloc(sizeof(*rv))) == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if ((rv = OPENSSL_zalloc(sizeof(*rv))) == NULL)
return NULL;
}
if (!sk_ASN1_STRING_TABLE_push(stable, rv)) {
OPENSSL_free(rv);
return NULL;
Expand Down Expand Up @@ -190,7 +188,7 @@ int ASN1_STRING_TABLE_add(int nid,

tmp = stable_get(nid);
if (tmp == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_ASN1_LIB);
return 0;
}
if (minsize >= 0)
Expand Down
4 changes: 1 addition & 3 deletions crypto/asn1/a_time.c
Expand Up @@ -420,10 +420,8 @@ int ASN1_TIME_set_string_X509(ASN1_TIME *s, const char *str)
* new t.data would be freed after ASN1_STRING_copy is done.
*/
t.data = OPENSSL_zalloc(t.length + 1);
if (t.data == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if (t.data == NULL)
goto out;
}
memcpy(t.data, str + 2, t.length);
t.type = V_ASN1_UTCTIME;
}
Expand Down
8 changes: 3 additions & 5 deletions crypto/asn1/a_verify.c
Expand Up @@ -33,7 +33,7 @@ int ASN1_verify(i2d_of_void *i2d, X509_ALGOR *a, ASN1_BIT_STRING *signature,
int ret = -1, i, inl;

if (ctx == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_EVP_LIB);
goto err;
}
i = OBJ_obj2nid(a->algorithm);
Expand All @@ -54,10 +54,8 @@ int ASN1_verify(i2d_of_void *i2d, X509_ALGOR *a, ASN1_BIT_STRING *signature,
goto err;
}
buf_in = OPENSSL_malloc((unsigned int)inl);
if (buf_in == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if (buf_in == NULL)
goto err;
}
p = buf_in;

i2d(data, &p);
Expand Down Expand Up @@ -206,7 +204,7 @@ int ASN1_item_verify_ctx(const ASN1_ITEM *it, const X509_ALGOR *alg,
goto err;
}
if (buf_in == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
ERR_raise(ERR_LIB_ASN1, ERR_R_ASN1_LIB);
goto err;
}
inll = inl;
Expand Down
5 changes: 1 addition & 4 deletions crypto/asn1/ameth_lib.c
Expand Up @@ -222,10 +222,8 @@ EVP_PKEY_ASN1_METHOD *EVP_PKEY_asn1_new(int id, int flags,
{
EVP_PKEY_ASN1_METHOD *ameth = OPENSSL_zalloc(sizeof(*ameth));

if (ameth == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
if (ameth == NULL)
return NULL;
}

ameth->pkey_id = id;
ameth->pkey_base_id = id;
Expand All @@ -247,7 +245,6 @@ EVP_PKEY_ASN1_METHOD *EVP_PKEY_asn1_new(int id, int flags,

err:
EVP_PKEY_asn1_free(ameth);
ERR_raise(ERR_LIB_ASN1, ERR_R_MALLOC_FAILURE);
return NULL;
}

Expand Down

0 comments on commit e077455

Please sign in to comment.