openssl3/contribs10/0104-CVE-2023-0215-UAF-bio.patch

From 8818064ce3c3c0f1b740a5aaba2a987e75bfbafd Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
Date: Wed, 14 Dec 2022 16:18:14 +0000
Subject: [PATCH 06/18] Fix a UAF resulting from a bug in BIO_new_NDEF

If the aux->asn1_cb() call fails in BIO_new_NDEF then the "out" BIO will
be part of an invalid BIO chain. This causes a "use after free" when the
BIO is eventually freed.

Based on an original patch by Viktor Dukhovni and an idea from Theo
Buehler.

Thanks to Octavio Galland for reporting this issue.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
---
 crypto/asn1/bio_ndef.c | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/crypto/asn1/bio_ndef.c b/crypto/asn1/bio_ndef.c
index d94e3a3644..b9df3a7a47 100644
--- a/crypto/asn1/bio_ndef.c
+++ b/crypto/asn1/bio_ndef.c
@@ -49,13 +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);
 
-/* unfortunately cannot constify this due to CMS_stream() and PKCS7_stream() */
+/*
+ * 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) {
         ERR_raise(ERR_LIB_ASN1, ASN1_R_STREAMING_NOT_SUPPORTED);
@@ -70,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;
@@ -92,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;
-- 
2.39.1

From f596ec8a6f9f5fcfa8e46a73b60f78a609725294 Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
Date: Wed, 14 Dec 2022 17:15:18 +0000
Subject: [PATCH 07/18] Check CMS failure during BIO setup with -stream is
 handled correctly

Test for the issue fixed in the previous commit

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
---
 test/recipes/80-test_cms.t  | 15 +++++++++++++--
 test/smime-certs/badrsa.pem | 18 ++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)
 create mode 100644 test/smime-certs/badrsa.pem

diff --git a/test/recipes/80-test_cms.t b/test/recipes/80-test_cms.t
index 610f1cbc51..fd53683e6b 100644
--- a/test/recipes/80-test_cms.t
+++ b/test/recipes/80-test_cms.t
@@ -13,7 +13,7 @@ use warnings;
 use POSIX;
 use File::Spec::Functions qw/catfile/;
 use File::Compare qw/compare_text compare/;
-use OpenSSL::Test qw/:DEFAULT srctop_dir srctop_file bldtop_dir bldtop_file/;
+use OpenSSL::Test qw/:DEFAULT srctop_dir srctop_file bldtop_dir bldtop_file with/;
 
 use OpenSSL::Test::Utils;
 
@@ -50,7 +50,7 @@ my ($no_des, $no_dh, $no_dsa, $no_ec, $no_ec2m, $no_rc2, $no_zlib)
 
 $no_rc2 = 1 if disabled("legacy");
 
-plan tests => 12;
+plan tests => 13;
 
 ok(run(test(["pkcs7_test"])), "test pkcs7");
 
@@ -972,3 +972,14 @@ ok(!run(app(['openssl', 'cms', '-verify',
 
     return "";
 }
+
+# Check that we get the expected failure return code
+with({ exit_checker => sub { return shift == 6; } },
+    sub {
+        ok(run(app(['openssl', 'cms', '-encrypt',
+                    '-in', srctop_file("test", "smcont.txt"),
+                    '-stream', '-recip',
+                    srctop_file("test/smime-certs", "badrsa.pem"),
+                   ])),
+            "Check failure during BIO setup with -stream is handled correctly");
+    });
diff --git a/test/smime-certs/badrsa.pem b/test/smime-certs/badrsa.pem
new file mode 100644
index 0000000000..f824fc2267
--- /dev/null
+++ b/test/smime-certs/badrsa.pem
@@ -0,0 +1,18 @@
+-----BEGIN CERTIFICATE-----
+MIIDbTCCAlWgAwIBAgIToTV4Z0iuK08vZP20oTh//hC8BDANBgkqhkiG9w0BAQ0FADAtMSswKQYD
+VfcDEyJTYW1wbGUgTEFNUFMgQ2VydGlmaWNhdGUgQXV0aG9yaXR5MCAXDTE5MTEyMDA2NTQxOFoY
+DzIwNTIwOTI3MDY1NDE4WjAZMRcwFQYDVQQDEw5BbGljZSBMb3ZlbGFjZTCCASIwDQYJKoZIhvcN
+AQEBBQADggEPADCCAQoCggEBALT0iehYOBY+TZp/T5K2KNI05Hwr+E3wP6XTvyi6WWyTgBK9LCOw
+I2juwdRrjFBmXkk7pWpjXwsA3A5GOtz0FpfgyC7OxsVcF7q4WHWZWleYXFKlQHJD73nQwXP968+A
+/3rBX7PhO0DBbZnfitOLPgPEwjTtdg0VQQ6Wz+CRQ/YbHPKaw7aRphZO63dKvIKp4cQVtkWQHi6s
+yTjGsgkLcLNau5LZDQUdsGV+SAo3nBdWCRYV+I65x8Kf4hCxqqmjV3d/2NKRu0BXnDe/N+iDz3X0
+zEoj0fqXgq4SWcC0nsG1lyyXt1TL270I6ATKRGJWiQVCCpDtc0NT6vdJ45bCSxgCAwEAAaOBlzCB
+lDAMBgNVHRMBAf8EAjAAMB4GA1UdEQQXMBWBE2FsaWNlQHNtaW1lLmV4YW1wbGUwEwYDVR0lBAww
+CgYIKwYBBQUHAwQwDwYDVR0PAQH/BAUDAwfAADAdBgNVHQ4EFgQUu/bMsi0dBhIcl64papAQ0yBm
+ZnMwHwYDVR0jBBgwFoAUeF8OWnjYa+RUcD2z3ez38fL6wEcwDQYJKoZIhvcNAQENBQADggEBABbW
+eonR6TMTckehDKNOabwaCIcekahAIL6l9tTzUX5ew6ufiAPlC6I/zQlmUaU0iSyFDG1NW14kNbFt
+5CAokyLhMtE4ASHBIHbiOp/ZSbUBTVYJZB61ot7w1/ol5QECSs08b8zrxIncf+t2DHGuVEy/Qq1d
+rBz8d4ay8zpqAE1tUyL5Da6ZiKUfWwZQXSI/JlbjQFzYQqTRDnzHWrg1xPeMTO1P2/cplFaseTiv
+yk4cYwOp/W9UAWymOZXF8WcJYCIUXkdcG/nEZxr057KlScrJmFXOoh7Y+8ON4iWYYcAfiNgpUFo/
+j8BAwrKKaFvdlZS9k1Ypb2+UQY75mKJE9Bg=
+-----END CERTIFICATE-----
-- 
2.39.1

1	From 8818064ce3c3c0f1b740a5aaba2a987e75bfbafd Mon Sep 17 00:00:00 2001
2	From: Matt Caswell <matt@openssl.org>
3	Date: Wed, 14 Dec 2022 16:18:14 +0000
4	Subject: [PATCH 06/18] Fix a UAF resulting from a bug in BIO_new_NDEF
5
6	If the aux->asn1_cb() call fails in BIO_new_NDEF then the "out" BIO will
7	be part of an invalid BIO chain. This causes a "use after free" when the
8	BIO is eventually freed.
9
10	Based on an original patch by Viktor Dukhovni and an idea from Theo
11	Buehler.
12
13	Thanks to Octavio Galland for reporting this issue.
14
15	Reviewed-by: Paul Dale <pauli@openssl.org>
16	Reviewed-by: Tomas Mraz <tomas@openssl.org>
17	---
18	crypto/asn1/bio_ndef.c \| 40 ++++++++++++++++++++++++++++++++--------
19	1 file changed, 32 insertions(+), 8 deletions(-)
20
21	diff --git a/crypto/asn1/bio_ndef.c b/crypto/asn1/bio_ndef.c
22	index d94e3a3644..b9df3a7a47 100644
23	--- a/crypto/asn1/bio_ndef.c
24	+++ b/crypto/asn1/bio_ndef.c
25	@@ -49,13 +49,19 @@ static int ndef_suffix(BIO b, unsigned char pbuf, int plen, void *parg);
26	static int ndef_suffix_free(BIO b, unsigned char pbuf, int plen,
27	void *parg);
28
29	-/* unfortunately cannot constify this due to CMS_stream() and PKCS7_stream() */
30	+/*
31	+ * On success, the returned BIO owns the input BIO as part of its BIO chain.
32	+ * On failure, NULL is returned and the input BIO is owned by the caller.
33	+ *
34	+ * Unfortunately cannot constify this due to CMS_stream() and PKCS7_stream()
35	+ */
36	BIO BIO_new_NDEF(BIO out, ASN1_VALUE val, const ASN1_ITEM it)
37	{
38	NDEF_SUPPORT *ndef_aux = NULL;
39	BIO *asn_bio = NULL;
40	const ASN1_AUX *aux = it->funcs;
41	ASN1_STREAM_ARG sarg;
42	+ BIO *pop_bio = NULL;
43
44	if (!aux \|\| !aux->asn1_cb) {
45	ERR_raise(ERR_LIB_ASN1, ASN1_R_STREAMING_NOT_SUPPORTED);
46	@@ -70,21 +76,39 @@ BIO BIO_new_NDEF(BIO out, ASN1_VALUE val, const ASN1_ITEM it)
47	out = BIO_push(asn_bio, out);
48	if (out == NULL)
49	goto err;
50	+ pop_bio = asn_bio;
51
52	- BIO_asn1_set_prefix(asn_bio, ndef_prefix, ndef_prefix_free);
53	- BIO_asn1_set_suffix(asn_bio, ndef_suffix, ndef_suffix_free);
54	+ if (BIO_asn1_set_prefix(asn_bio, ndef_prefix, ndef_prefix_free) <= 0
55	+ \|\| BIO_asn1_set_suffix(asn_bio, ndef_suffix, ndef_suffix_free) <= 0
56	+ \|\| BIO_ctrl(asn_bio, BIO_C_SET_EX_ARG, 0, ndef_aux) <= 0)
57	+ goto err;
58
59	/*
60	- * Now let callback prepends any digest, cipher etc BIOs ASN1 structure
61	- * needs.
62	+ * Now let the callback prepend any digest, cipher, etc., that the BIO's
63	+ * ASN1 structure needs.
64	*/
65
66	sarg.out = out;
67	sarg.ndef_bio = NULL;
68	sarg.boundary = NULL;
69
70	- if (aux->asn1_cb(ASN1_OP_STREAM_PRE, &val, it, &sarg) <= 0)
71	+ /*
72	+ * The asn1_cb(), must not have mutated asn_bio on error, leaving it in the
73	+ * middle of some partially built, but not returned BIO chain.
74	+ */
75	+ if (aux->asn1_cb(ASN1_OP_STREAM_PRE, &val, it, &sarg) <= 0) {
76	+ /*
77	+ * ndef_aux is now owned by asn_bio so we must not free it in the err
78	+ * clean up block
79	+ */
80	+ ndef_aux = NULL;
81	goto err;
82	+ }
83	+
84	+ /*
85	+ * We must not fail now because the callback has prepended additional
86	+ * BIOs to the chain
87	+ */
88
89	ndef_aux->val = val;
90	ndef_aux->it = it;
91	@@ -92,11 +116,11 @@ BIO BIO_new_NDEF(BIO out, ASN1_VALUE val, const ASN1_ITEM it)
92	ndef_aux->boundary = sarg.boundary;
93	ndef_aux->out = out;
94
95	- BIO_ctrl(asn_bio, BIO_C_SET_EX_ARG, 0, ndef_aux);
96	-
97	return sarg.ndef_bio;
98
99	err:
100	+ /* BIO_pop() is NULL safe */
101	+ (void)BIO_pop(pop_bio);
102	BIO_free(asn_bio);
103	OPENSSL_free(ndef_aux);
104	return NULL;
105	--
106	2.39.1
107
108	From f596ec8a6f9f5fcfa8e46a73b60f78a609725294 Mon Sep 17 00:00:00 2001
109	From: Matt Caswell <matt@openssl.org>
110	Date: Wed, 14 Dec 2022 17:15:18 +0000
111	Subject: [PATCH 07/18] Check CMS failure during BIO setup with -stream is
112	handled correctly
113
114	Test for the issue fixed in the previous commit
115
116	Reviewed-by: Paul Dale <pauli@openssl.org>
117	Reviewed-by: Tomas Mraz <tomas@openssl.org>
118	---
119	test/recipes/80-test_cms.t \| 15 +++++++++++++--
120	test/smime-certs/badrsa.pem \| 18 ++++++++++++++++++
121	2 files changed, 31 insertions(+), 2 deletions(-)
122	create mode 100644 test/smime-certs/badrsa.pem
123
124	diff --git a/test/recipes/80-test_cms.t b/test/recipes/80-test_cms.t
125	index 610f1cbc51..fd53683e6b 100644
126	--- a/test/recipes/80-test_cms.t
127	+++ b/test/recipes/80-test_cms.t
128	@@ -13,7 +13,7 @@ use warnings;
129	use POSIX;
130	use File::Spec::Functions qw/catfile/;
131	use File::Compare qw/compare_text compare/;
132	-use OpenSSL::Test qw/:DEFAULT srctop_dir srctop_file bldtop_dir bldtop_file/;
133	+use OpenSSL::Test qw/:DEFAULT srctop_dir srctop_file bldtop_dir bldtop_file with/;
134
135	use OpenSSL::Test::Utils;
136
137	@@ -50,7 +50,7 @@ my ($no_des, $no_dh, $no_dsa, $no_ec, $no_ec2m, $no_rc2, $no_zlib)
138
139	$no_rc2 = 1 if disabled("legacy");
140
141	-plan tests => 12;
142	+plan tests => 13;
143
144	ok(run(test(["pkcs7_test"])), "test pkcs7");
145
146	@@ -972,3 +972,14 @@ ok(!run(app(['openssl', 'cms', '-verify',
147
148	return "";
149	}
150	+
151	+# Check that we get the expected failure return code
152	+with({ exit_checker => sub { return shift == 6; } },
153	+ sub {
154	+ ok(run(app(['openssl', 'cms', '-encrypt',
155	+ '-in', srctop_file("test", "smcont.txt"),
156	+ '-stream', '-recip',
157	+ srctop_file("test/smime-certs", "badrsa.pem"),
158	+ ])),
159	+ "Check failure during BIO setup with -stream is handled correctly");
160	+ });
161	diff --git a/test/smime-certs/badrsa.pem b/test/smime-certs/badrsa.pem
162	new file mode 100644
163	index 0000000000..f824fc2267
164	--- /dev/null
165	+++ b/test/smime-certs/badrsa.pem
166	@@ -0,0 +1,18 @@
167	+-----BEGIN CERTIFICATE-----
168	+MIIDbTCCAlWgAwIBAgIToTV4Z0iuK08vZP20oTh//hC8BDANBgkqhkiG9w0BAQ0FADAtMSswKQYD
169	+VfcDEyJTYW1wbGUgTEFNUFMgQ2VydGlmaWNhdGUgQXV0aG9yaXR5MCAXDTE5MTEyMDA2NTQxOFoY
170	+DzIwNTIwOTI3MDY1NDE4WjAZMRcwFQYDVQQDEw5BbGljZSBMb3ZlbGFjZTCCASIwDQYJKoZIhvcN
171	+AQEBBQADggEPADCCAQoCggEBALT0iehYOBY+TZp/T5K2KNI05Hwr+E3wP6XTvyi6WWyTgBK9LCOw
172	+I2juwdRrjFBmXkk7pWpjXwsA3A5GOtz0FpfgyC7OxsVcF7q4WHWZWleYXFKlQHJD73nQwXP968+A
173	+/3rBX7PhO0DBbZnfitOLPgPEwjTtdg0VQQ6Wz+CRQ/YbHPKaw7aRphZO63dKvIKp4cQVtkWQHi6s
174	+yTjGsgkLcLNau5LZDQUdsGV+SAo3nBdWCRYV+I65x8Kf4hCxqqmjV3d/2NKRu0BXnDe/N+iDz3X0
175	+zEoj0fqXgq4SWcC0nsG1lyyXt1TL270I6ATKRGJWiQVCCpDtc0NT6vdJ45bCSxgCAwEAAaOBlzCB
176	+lDAMBgNVHRMBAf8EAjAAMB4GA1UdEQQXMBWBE2FsaWNlQHNtaW1lLmV4YW1wbGUwEwYDVR0lBAww
177	+CgYIKwYBBQUHAwQwDwYDVR0PAQH/BAUDAwfAADAdBgNVHQ4EFgQUu/bMsi0dBhIcl64papAQ0yBm
178	+ZnMwHwYDVR0jBBgwFoAUeF8OWnjYa+RUcD2z3ez38fL6wEcwDQYJKoZIhvcNAQENBQADggEBABbW
179	+eonR6TMTckehDKNOabwaCIcekahAIL6l9tTzUX5ew6ufiAPlC6I/zQlmUaU0iSyFDG1NW14kNbFt
180	+5CAokyLhMtE4ASHBIHbiOp/ZSbUBTVYJZB61ot7w1/ol5QECSs08b8zrxIncf+t2DHGuVEy/Qq1d
181	+rBz8d4ay8zpqAE1tUyL5Da6ZiKUfWwZQXSI/JlbjQFzYQqTRDnzHWrg1xPeMTO1P2/cplFaseTiv
182	+yk4cYwOp/W9UAWymOZXF8WcJYCIUXkdcG/nEZxr057KlScrJmFXOoh7Y+8ON4iWYYcAfiNgpUFo/
183	+j8BAwrKKaFvdlZS9k1Ypb2+UQY75mKJE9Bg=
184	+-----END CERTIFICATE-----
185	--
186	2.39.1
187