OID code tidy up.

Tidy up and simplify OBJ_dup() and OBJ_create().

Sanity check added OIDs: don't allow duplicates.

Reviewed-by: Richard Levitte <levitte@openssl.org>

3 files changed, 48 insertions, 58 deletions

diff --git a/crypto/objects/obj_dat.c b/crypto/objects/obj_dat.c
index 9b8e93b054..820c275fd0 100644
--- a/crypto/objects/obj_dat.c
+++ b/crypto/objects/obj_dat.c
@@ -679,30 +679,36 @@ int OBJ_create_objects(BIO *in)
 
 int OBJ_create(const char *oid, const char *sn, const char *ln)
 {
-    int ok = 0;
-    ASN1_OBJECT *op = NULL;
-    unsigned char *buf;
-    int i;
-
-    i = a2d_ASN1_OBJECT(NULL, 0, oid, -1);
-    if (i <= 0)
-        return (0);
+    ASN1_OBJECT *tmpoid = NULL;
+    int ok;
 
-    if ((buf = OPENSSL_malloc(i)) == NULL) {
-        OBJerr(OBJ_F_OBJ_CREATE, ERR_R_MALLOC_FAILURE);
-        return (0);
+    /* Check to see if short or long name already present */
+    if (OBJ_sn2nid(sn) != NID_undef || OBJ_ln2nid(ln) != NID_undef) {
+        OBJerr(OBJ_F_OBJ_CREATE, OBJ_R_OID_EXISTS);
+        return 0;
     }
-    i = a2d_ASN1_OBJECT(buf, i, oid, -1);
-    if (i == 0)
-        goto err;
-    op = (ASN1_OBJECT *)ASN1_OBJECT_create(OBJ_new_nid(1), buf, i, sn, ln);
-    if (op == NULL)
+
+    /* Convert numerical OID string to an ASN1_OBJECT structure */
+    tmpoid = OBJ_txt2obj(oid, 1);
+
+    /* If NID is not NID_undef then object already exists */
+    if (OBJ_obj2nid(tmpoid) != NID_undef) {
+        OBJerr(OBJ_F_OBJ_CREATE, OBJ_R_OID_EXISTS);
         goto err;
-    ok = OBJ_add_object(op);
+    }
+
+    tmpoid->nid = OBJ_new_nid(1);
+    tmpoid->sn = (char *)sn;
+    tmpoid->ln = (char *)ln;
+
+    ok = OBJ_add_object(tmpoid);
+
+    tmpoid->sn = NULL;
+    tmpoid->ln = NULL;
+
  err:
-    ASN1_OBJECT_free(op);
-    OPENSSL_free(buf);
-    return (ok);
+    ASN1_OBJECT_free(tmpoid);
+    return ok;
 }
 
 size_t OBJ_length(const ASN1_OBJECT *obj)
diff --git a/crypto/objects/obj_err.c b/crypto/objects/obj_err.c
index cadfe81fbc..b8a90573ad 100644
--- a/crypto/objects/obj_err.c
+++ b/crypto/objects/obj_err.c
@@ -36,6 +36,7 @@ static ERR_STRING_DATA OBJ_str_functs[] = {
 
 static ERR_STRING_DATA OBJ_str_reasons[] = {
     {ERR_REASON(OBJ_R_MALLOC_FAILURE), "malloc failure"},
+    {ERR_REASON(OBJ_R_OID_EXISTS), "oid exists"},
     {ERR_REASON(OBJ_R_UNKNOWN_NID), "unknown nid"},
     {0, NULL}
 };
diff --git a/crypto/objects/obj_lib.c b/crypto/objects/obj_lib.c
index da977fedc8..33075e6451 100644
--- a/crypto/objects/obj_lib.c
+++ b/crypto/objects/obj_lib.c
@@ -17,59 +17,42 @@
 ASN1_OBJECT *OBJ_dup(const ASN1_OBJECT *o)
 {
     ASN1_OBJECT *r;
-    int i;
-    char *ln = NULL, *sn = NULL;
-    unsigned char *data = NULL;
 
     if (o == NULL)
-        return (NULL);
+        return NULL;
+    /* If object isn't dynamic it's an internal OID which is never freed */
     if (!(o->flags & ASN1_OBJECT_FLAG_DYNAMIC))
-        return ((ASN1_OBJECT *)o); /* XXX: ugh! Why? What kind of duplication
-                                    * is this??? */
+        return ((ASN1_OBJECT *)o);
 
     r = ASN1_OBJECT_new();
     if (r == NULL) {
         OBJerr(OBJ_F_OBJ_DUP, ERR_R_ASN1_LIB);
         return (NULL);
     }
-    data = OPENSSL_malloc(o->length);
-    if (data == NULL)
-        goto err;
-    if (o->data != NULL)
-        memcpy(data, o->data, o->length);
-    /* once data attached to object it remains const */
-    r->data = data;
-    r->length = o->length;
-    r->nid = o->nid;
-    r->ln = r->sn = NULL;
-    if (o->ln != NULL) {
-        i = strlen(o->ln) + 1;
-        ln = OPENSSL_malloc(i);
-        if (ln == NULL)
-            goto err;
-        memcpy(ln, o->ln, i);
-        r->ln = ln;
-    }
 
-    if (o->sn != NULL) {
-        i = strlen(o->sn) + 1;
-        sn = OPENSSL_malloc(i);
-        if (sn == NULL)
-            goto err;
-        memcpy(sn, o->sn, i);
-        r->sn = sn;
-    }
+    /* Set dynamic flags so everything gets freed up on error */
+
     r->flags = o->flags | (ASN1_OBJECT_FLAG_DYNAMIC |
                            ASN1_OBJECT_FLAG_DYNAMIC_STRINGS |
                            ASN1_OBJECT_FLAG_DYNAMIC_DATA);
-    return (r);
+
+    if (o->length > 0 && (r->data = OPENSSL_memdup(o->data, o->length)) == NULL)
+        goto err;
+
+    r->length = o->length;
+    r->nid = o->nid;
+
+    if (o->ln != NULL && (r->ln = OPENSSL_strdup(o->ln)) == NULL)
+        goto err;
+
+    if (o->sn != NULL && (r->sn = OPENSSL_strdup(o->sn)) == NULL)
+        goto err;
+
+    return r;
  err:
+    ASN1_OBJECT_free(r);
     OBJerr(OBJ_F_OBJ_DUP, ERR_R_MALLOC_FAILURE);
-    OPENSSL_free(ln);
-    OPENSSL_free(sn);
-    OPENSSL_free(data);
-    OPENSSL_free(r);
-    return (NULL);
+    return NULL;
 }
 
 int OBJ_cmp(const ASN1_OBJECT *a, const ASN1_OBJECT *b)