Fix bugs in X509_NAME_ENTRY_set

The wrong "set" field was incremented in the wrong place and would
create a new RDN, not a multi-valued RDN.
RDN inserts would happen after not before.
Prepending an entry to an RDN incorrectly created a new RDN

Anything which built up an X509_NAME could get a messed-up structure,
which would then be "wrong" for anyone using that name.

Thanks to Ingo Schwarze for extensive debugging and the initial
fix (documented in GitHub issue #5870).

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/5882)

diff --git a/crypto/x509/x509name.c b/crypto/x509/x509name.c
index bde5db4066128e78671c9fbb8ed99a8131a1208c..8b08cae2317678babbec7abba39e5dd32ebab0f7 100644 (file)
--- a/crypto/x509/x509name.c
+++ b/crypto/x509/x509name.c
@@ -193,7 +193,7 @@ int X509_NAME_add_entry(X509_NAME *name, const X509_NAME_ENTRY *ne, int loc,
         loc = n;
     else if (loc < 0)
         loc = n;
-
+    inc = (set == 0);
     name->modified = 1;
 
     if (set == -1) {
@@ -202,7 +202,6 @@ int X509_NAME_add_entry(X509_NAME *name, const X509_NAME_ENTRY *ne, int loc,
             inc = 1;
         } else {
             set = sk_X509_NAME_ENTRY_value(sk, loc - 1)->set;
-            inc = 0;
         }
     } else {                    /* if (set >= 0) */
 
@@ -213,12 +212,11 @@ int X509_NAME_add_entry(X509_NAME *name, const X509_NAME_ENTRY *ne, int loc,
                 set = 0;
         } else
             set = sk_X509_NAME_ENTRY_value(sk, loc)->set;
-        inc = (set == 0) ? 1 : 0;
     }
 
     /*
      * X509_NAME_ENTRY_dup is ASN1 generated code, that can't be easily
-     * const'ified; harmless cast as dup() don't modify its input.
+     * const'ified; harmless cast since dup() don't modify its input.
      */
     if ((new_name = X509_NAME_ENTRY_dup((X509_NAME_ENTRY *)ne)) == NULL)
         goto err;
@@ -230,7 +228,7 @@ int X509_NAME_add_entry(X509_NAME *name, const X509_NAME_ENTRY *ne, int loc,
     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;
  err: