Fix two bugs in X509_NAME_add_entry(3):

(1) Evaluate the "set" argument, which says whether to create a new
RDN or to prepend or append to an existing one, before reusing it
for a different purpose, i.e. for the "set" field of the new
X509_NAME_ENTRY structure.
(2) When incrementing of some "set" fields is needed, increment the
correct ones: All those to the right of the newly inserted entry,
but not the one of that entry itself.

These two bugs caused wrong results whenever using loc != -1,
i.e. whenever inserting rather than appending entries, even when
using set == 0 only, that is, even when using single-values RDNs only.

Both bugs have been continuously present since at least SSLeay-0.8.1
(released July 18, 1997) and the second one since at least SSLeay-0.8.0
(released June 25, 1997), so both are over twenty years old.

I found these bugs by code inspection while trying to document the
function X509_NAME_ENTRY_set(3), which is public, but undocumented
in OpenSSL.

OK beck@, jsing@

1 files changed, 4 insertions, 7 deletions

diff --git a/src/lib/libcrypto/x509/x509name.c b/src/lib/libcrypto/x509/x509name.c
index 2ca1a76b64..4e2695fd74 100644
--- a/src/lib/libcrypto/x509/x509name.c
+++ b/src/lib/libcrypto/x509/x509name.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509name.c,v 1.15 2018/03/17 15:28:27 tb Exp $ */
+/* $OpenBSD: x509name.c,v 1.16 2018/04/04 11:59:26 schwarze Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -249,17 +249,15 @@ X509_NAME_add_entry(X509_NAME *name, X509_NAME_ENTRY *ne, int loc, int set)
                 loc = n;
         else if (loc < 0)
                 loc = n;
-
+        inc = (set == 0);
         name->modified = 1;
 
         if (set == -1) {
                 if (loc == 0) {
                         set = 0;
                         inc = 1;
-                } else {
+                } else
                         set = sk_X509_NAME_ENTRY_value(sk, loc - 1)->set;
-                        inc = 0;
-                }
         } else /* if (set >= 0) */ {
                 if (loc >= n) {
                         if (loc != 0)
@@ -268,7 +266,6 @@ X509_NAME_add_entry(X509_NAME *name, X509_NAME_ENTRY *ne, int loc, int set)
                                 set = 0;
                 } else
                         set = sk_X509_NAME_ENTRY_value(sk, loc)->set;
-                inc = (set == 0) ? 1 : 0;
         }
 
         if ((new_name = X509_NAME_ENTRY_dup(ne)) == NULL)
@@ -281,7 +278,7 @@ X509_NAME_add_entry(X509_NAME *name, X509_NAME_ENTRY *ne, int loc, int set)
         if (inc) {
                 n = sk_X509_NAME_ENTRY_num(sk);
                 for (i = loc + 1; i < n; i++)
-                        sk_X509_NAME_ENTRY_value(sk, i - 1)->set += 1;
+                        sk_X509_NAME_ENTRY_value(sk, i)->set += 1;
         }
         return (1);