Harden asn1 oid loader to invalid inputs

In the event that a config file contains this sequence:
=======
openssl_conf = openssl_init

config_diagnostics = 1

[openssl_init]
oid_section = oids

[oids]
testoid1 = 1.2.3.4.1
testoid2 = A Very Long OID Name, 1.2.3.4.2
testoid3 = ,1.2.3.4.3
======

The leading comma in testoid3 can cause a heap buffer overflow, as the
parsing code will move the string pointer back 1 character, thereby
pointing to an invalid memory space

correct the parser to detect this condition and handle it by treating it
as if the comma doesn't exist (i.e. an empty long oid name)

(cherry picked from commit a552c23c6502592c1b3c67d93dd7e5ffbe958aa4)

Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23034)

(cherry picked from commit d802bfbf80bab00123a4a6209f255852b3a10207)

diff --git a/crypto/asn1/asn_moid.c b/crypto/asn1/asn_moid.c
index 526219c1a723c98e66c48430000d80b3b76289a6..9aaab8a269d30d390031f2e6bdf388127b6ce8cb 100644 (file)
--- a/crypto/asn1/asn_moid.c
+++ b/crypto/asn1/asn_moid.c
@@ -67,6 +67,10 @@ static int do_create(const char *value, const char *name)
     if (p == NULL) {
         ln = name;
         ostr = value;
+    } else if (p == value) {
+        /* we started with a leading comma */
+        ln = name;
+        ostr = p + 1;
     } else {
         ln = value;
         ostr = p + 1;

diff --git a/test/recipes/04-test_asn1_parse.t b/test/recipes/04-test_asn1_parse.t
new file mode 100644 (file)
index 0000000..f3af436
--- /dev/null
+++ b/test/recipes/04-test_asn1_parse.t
@@ -0,0 +1,26 @@
+#! /usr/bin/env perl
+# Copyright 2023 The OpenSSL Project Authors. All Rights Reserved.
+#
+# Licensed under the Apache License 2.0 (the "License").  You may not use
+# this file except in compliance with the License.  You can obtain a copy
+# in the file LICENSE in the source distribution or at
+# https://www.openssl.org/source/license.html
+
+use strict;
+use OpenSSL::Test qw(:DEFAULT srctop_file);
+use OpenSSL::Test::Utils;
+
+setup("test_asn1_parse");
+
+plan tests => 3;
+
+$ENV{OPENSSL_CONF} = srctop_file("test", "test_asn1_parse.cnf");
+
+ok(run(app(([ 'openssl', 'asn1parse',
+              '-genstr', 'OID:1.2.3.4.1']))));
+
+ok(run(app(([ 'openssl', 'asn1parse',
+              '-genstr', 'OID:1.2.3.4.2']))));
+
+ok(run(app(([ 'openssl', 'asn1parse',
+              '-genstr', 'OID:1.2.3.4.3']))));

diff --git a/test/test_asn1_parse.cnf b/test/test_asn1_parse.cnf
new file mode 100644 (file)
index 0000000..5f03056
--- /dev/null
+++ b/test/test_asn1_parse.cnf
@@ -0,0 +1,12 @@
+openssl_conf = openssl_init
+
+# Comment out the next line to ignore configuration errors
+config_diagnostics = 1
+
+[openssl_init]
+oid_section = oids
+
+[oids]
+testoid1 = 1.2.3.4.1
+testoid2 = A Very Long OID Name, 1.2.3.4.2
+testoid3 = ,1.2.3.4.3