3 files changed, 40 insertions, 11 deletions

diff --git a/crypto/asn1/asn_mime.c b/crypto/asn1/asn_mime.c
index 38735cd86f3e..36853612b69c 100644
--- a/crypto/asn1/asn_mime.c
+++ b/crypto/asn1/asn_mime.c
@@ -489,6 +489,7 @@ int SMIME_crlf_copy(BIO *in, BIO *out, int flags)
     char eol;
     int len;
     char linebuf[MAX_SMLEN];
+    int ret;
     /*
      * Buffer output so we don't write one line at a time. This is useful
      * when streaming as we don't end up with one OCTET STRING per line.
@@ -523,9 +524,12 @@ int SMIME_crlf_copy(BIO *in, BIO *out, int flags)
                 BIO_write(out, "\r\n", 2);
         }
     }
-    (void)BIO_flush(out);
+    ret = BIO_flush(out);
     BIO_pop(out);
     BIO_free(bf);
+    if (ret <= 0)
+        return 0;
+
     return 1;
 }
 
diff --git a/crypto/asn1/bio_asn1.c b/crypto/asn1/bio_asn1.c
index 914d77c866c6..17b0d1aa6cad 100644
--- a/crypto/asn1/bio_asn1.c
+++ b/crypto/asn1/bio_asn1.c
@@ -172,7 +172,7 @@ static int asn1_bio_write(BIO *b, const char *in, int inl)
         case ASN1_STATE_START:
             if (!asn1_bio_setup_ex(b, ctx, ctx->prefix,
                                    ASN1_STATE_PRE_COPY, ASN1_STATE_HEADER))
-                return 0;
+                return -1;
             break;
 
             /* Copy any pre data first */
@@ -189,7 +189,7 @@ static int asn1_bio_write(BIO *b, const char *in, int inl)
         case ASN1_STATE_HEADER:
             ctx->buflen = ASN1_object_size(0, inl, ctx->asn1_tag) - inl;
             if (!ossl_assert(ctx->buflen <= ctx->bufsize))
-                return 0;
+                return -1;
             p = ctx->buf;
             ASN1_put_object(&p, 0, inl, ctx->asn1_tag, ctx->asn1_class);
             ctx->copylen = inl;
diff --git a/crypto/asn1/bio_ndef.c b/crypto/asn1/bio_ndef.c
index 760e4846a474..c8a776b482d0 100644
--- a/crypto/asn1/bio_ndef.c
+++ b/crypto/asn1/bio_ndef.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2021 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2008-2023 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -49,12 +49,19 @@ static int ndef_suffix(BIO *b, unsigned char **pbuf, int *plen, void *parg);
 static int ndef_suffix_free(BIO *b, unsigned char **pbuf, int *plen,
                             void *parg);
 
+/*
+ * On success, the returned BIO owns the input BIO as part of its BIO chain.
+ * On failure, NULL is returned and the input BIO is owned by the caller.
+ *
+ * Unfortunately cannot constify this due to CMS_stream() and PKCS7_stream()
+ */
 BIO *BIO_new_NDEF(BIO *out, ASN1_VALUE *val, const ASN1_ITEM *it)
 {
     NDEF_SUPPORT *ndef_aux = NULL;
     BIO *asn_bio = NULL;
     const ASN1_AUX *aux = it->funcs;
     ASN1_STREAM_ARG sarg;
+    BIO *pop_bio = NULL;
 
     if (!aux || !aux->asn1_cb) {
         ASN1err(ASN1_F_BIO_NEW_NDEF, ASN1_R_STREAMING_NOT_SUPPORTED);
@@ -69,21 +76,39 @@ BIO *BIO_new_NDEF(BIO *out, ASN1_VALUE *val, const ASN1_ITEM *it)
     out = BIO_push(asn_bio, out);
     if (out == NULL)
         goto err;
+    pop_bio = asn_bio;
 
-    BIO_asn1_set_prefix(asn_bio, ndef_prefix, ndef_prefix_free);
-    BIO_asn1_set_suffix(asn_bio, ndef_suffix, ndef_suffix_free);
+    if (BIO_asn1_set_prefix(asn_bio, ndef_prefix, ndef_prefix_free) <= 0
+            || BIO_asn1_set_suffix(asn_bio, ndef_suffix, ndef_suffix_free) <= 0
+            || BIO_ctrl(asn_bio, BIO_C_SET_EX_ARG, 0, ndef_aux) <= 0)
+        goto err;
 
     /*
-     * Now let callback prepends any digest, cipher etc BIOs ASN1 structure
-     * needs.
+     * Now let the callback prepend any digest, cipher, etc., that the BIO's
+     * ASN1 structure needs.
      */
 
     sarg.out = out;
     sarg.ndef_bio = NULL;
     sarg.boundary = NULL;
 
-    if (aux->asn1_cb(ASN1_OP_STREAM_PRE, &val, it, &sarg) <= 0)
+    /*
+     * The asn1_cb(), must not have mutated asn_bio on error, leaving it in the
+     * middle of some partially built, but not returned BIO chain.
+     */
+    if (aux->asn1_cb(ASN1_OP_STREAM_PRE, &val, it, &sarg) <= 0) {
+        /*
+         * ndef_aux is now owned by asn_bio so we must not free it in the err
+         * clean up block
+         */
+        ndef_aux = NULL;
         goto err;
+    }
+
+    /*
+     * We must not fail now because the callback has prepended additional
+     * BIOs to the chain
+     */
 
     ndef_aux->val = val;
     ndef_aux->it = it;
@@ -91,11 +116,11 @@ BIO *BIO_new_NDEF(BIO *out, ASN1_VALUE *val, const ASN1_ITEM *it)
     ndef_aux->boundary = sarg.boundary;
     ndef_aux->out = out;
 
-    BIO_ctrl(asn_bio, BIO_C_SET_EX_ARG, 0, ndef_aux);
-
     return sarg.ndef_bio;
 
  err:
+    /* BIO_pop() is NULL safe */
+    (void)BIO_pop(pop_bio);
     BIO_free(asn_bio);
     OPENSSL_free(ndef_aux);
     return NULL;