Code review

- make `wc_ecc_export_point_der_compressed` a local function
- use `int` for `shortKeySize` in `wc_ecc_import_point_der_ex`
- check for null return value from `wolfSSL_OBJ_nid2obj` and `wolfSSL_d2i_PUBKEY`
- add comments to `ssl.c`
- check `lnlen` in `wolfSSL_OBJ_ln2nid`

src/internal.c

@@ -9538,12 +9538,17 @@ int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert)
             } else {
                 wolfSSL_ASN1_OBJECT_free(x509->key.algor->algorithm);
             }
-            x509->key.algor->algorithm = wolfSSL_OBJ_nid2obj(dCert->keyOID);
+            if (!(x509->key.algor->algorithm =
+                    wolfSSL_OBJ_nid2obj(dCert->keyOID))) {
+                ret = PUBLIC_KEY_E;
+            }
 
             wolfSSL_EVP_PKEY_free(x509->key.pkey);
-            x509->key.pkey = wolfSSL_d2i_PUBKEY(NULL,
+            if (!(x509->key.pkey = wolfSSL_d2i_PUBKEY(NULL,
-                                            &dCert->publicKey,
+                                                      &dCert->publicKey,
-                                            dCert->pubKeySize);
+                                                      dCert->pubKeySize))) {
+                ret = PUBLIC_KEY_E;
+            }
         }
 #endif
     }
@@ -9562,7 +9567,10 @@ int CopyDecodedToX509(WOLFSSL_X509* x509, DecodedCert* dCert)
         }
 #if defined(OPENSSL_ALL)
         wolfSSL_ASN1_OBJECT_free(x509->algor.algorithm);
-        x509->algor.algorithm = wolfSSL_OBJ_nid2obj(dCert->signatureOID);
+        if (!(x509->algor.algorithm =
+                wolfSSL_OBJ_nid2obj(dCert->signatureOID))) {
+            ret = PUBLIC_KEY_E;
+        }
 #endif
     }
 

src/ssl.c

@@ -28179,6 +28179,16 @@ void wolfSSL_X509_ALGOR_get0(const WOLFSSL_ASN1_OBJECT **paobj, int *pptype,
     }
 }
 
+/**
+ * Populate algor members.
+ *
+ * @param algor The object to be set
+ * @param aobj The value to be set in algor->algorithm
+ * @param ptype The type of algor->parameter
+ * @param pval The value of algor->parameter
+ * @return WOLFSSL_SUCCESS on success
+ *         WOLFSSL_FAILURE on missing parameters or bad malloc
+ */
 int wolfSSL_X509_ALGOR_set0(WOLFSSL_X509_ALGOR *algor, WOLFSSL_ASN1_OBJECT *aobj,
                             int ptype, void *pval)
 {
@@ -28200,6 +28210,13 @@ int wolfSSL_X509_ALGOR_set0(WOLFSSL_X509_ALGOR *algor, WOLFSSL_ASN1_OBJECT *aobj
     return WOLFSSL_SUCCESS;
 }
 
+/**
+ * Set `a` in a smart way.
+ *
+ * @param a Object to set
+ * @param type The type of object in value
+ * @param value Object to set
+ */
 void wolfSSL_ASN1_TYPE_set(WOLFSSL_ASN1_TYPE *a, int type, void *value)
 {
     if (!a || !value) {
@@ -28222,6 +28239,11 @@ void wolfSSL_ASN1_TYPE_set(WOLFSSL_ASN1_TYPE *a, int type, void *value)
     a->type = type;
 }
 
+/**
+ * Allocate a new WOLFSSL_ASN1_TYPE object.
+ *
+ * @return New zero'ed WOLFSSL_ASN1_TYPE object
+ */
 WOLFSSL_ASN1_TYPE* wolfSSL_ASN1_TYPE_new(void)
 {
     WOLFSSL_ASN1_TYPE* ret = (WOLFSSL_ASN1_TYPE*)XMALLOC(sizeof(WOLFSSL_ASN1_TYPE),
@@ -28232,6 +28254,11 @@ WOLFSSL_ASN1_TYPE* wolfSSL_ASN1_TYPE_new(void)
     return ret;
 }
 
+/**
+ * Free WOLFSSL_ASN1_TYPE and all its members.
+ *
+ * @param at Object to free
+ */
 void wolfSSL_ASN1_TYPE_free(WOLFSSL_ASN1_TYPE* at)
 {
     if (at) {
@@ -28253,6 +28280,11 @@ void wolfSSL_ASN1_TYPE_free(WOLFSSL_ASN1_TYPE* at)
     }
 }
 
+/**
+ * Allocate a new WOLFSSL_X509_PUBKEY object.
+ *
+ * @return New zero'ed WOLFSSL_X509_PUBKEY object
+ */
 WOLFSSL_X509_PUBKEY *wolfSSL_X509_PUBKEY_new(void)
 {
     WOLFSSL_X509_PUBKEY *ret;
@@ -28270,6 +28302,11 @@ WOLFSSL_X509_PUBKEY *wolfSSL_X509_PUBKEY_new(void)
     return ret;
 }
 
+/**
+ * Free WOLFSSL_X509_PUBKEY and all its members.
+ *
+ * @param at Object to free
+ */
 void wolfSSL_X509_PUBKEY_free(WOLFSSL_X509_PUBKEY *x)
 {
     if (x) {
@@ -31614,6 +31651,8 @@ int wolfSSL_ASN1_item_i2d(const void *src, byte **dest,
         *dest = buf;
     }
     else if (dest && *dest && buf) {
+        /* *dest length is not checked because the user is responsible
+         * for providing a long enough buffer */
         XMEMCPY(*dest, buf, len);
     }
 
@@ -42892,13 +42931,15 @@ err:
                 ln++;
                 lnlen--;
             }
-            if (ln[lnlen-1] == '=') {
+            if (lnlen) {
-                lnlen--;
+                if (ln[lnlen-1] == '=') {
-            }
+                    lnlen--;
-            for (i = 0; i < WOLFSSL_OBJECT_INFO_SZ; i++, obj_info++) {
+                }
-                if (lnlen == XSTRLEN(obj_info->lName) &&
+                for (i = 0; i < WOLFSSL_OBJECT_INFO_SZ; i++, obj_info++) {
-                        XSTRNCMP(ln, obj_info->lName, lnlen) == 0) {
+                    if (lnlen == XSTRLEN(obj_info->lName) &&
-                    return obj_info->nid;
+                            XSTRNCMP(ln, obj_info->lName, lnlen) == 0) {
+                        return obj_info->nid;
+                    }
                 }
             }
         }

wolfcrypt/src/ecc.c

@@ -6287,7 +6287,7 @@ int wc_ecc_verify_hash_ex(mp_int *r, mp_int *s, const byte* hash,
 /* import point from der
  * if shortKeySize != 0 then keysize is always (inLen-1)>>1 */
 int wc_ecc_import_point_der_ex(byte* in, word32 inLen, const int curve_idx,
-                               ecc_point* point, char shortKeySize)
+                               ecc_point* point, int shortKeySize)
 {
     int err = 0;
 #ifdef HAVE_COMP_KEY

wolfssl/openssl/asn1.h

@@ -83,7 +83,9 @@ WOLFSSL_API WOLFSSL_ASN1_INTEGER *wolfSSL_BN_to_ASN1_INTEGER(
 WOLFSSL_API void wolfSSL_ASN1_TYPE_set(WOLFSSL_ASN1_TYPE *a, int type, void *value);
 
 #ifdef OPENSSL_ALL
-/* IMPLEMENT_ASN1_FUNCTIONS stuff */
+/* IMPLEMENT_ASN1_FUNCTIONS is strictly for external use only. Internally
+ * we don't use this. Some projects use OpenSSL to implement ASN1 types and
+ * this section is only to provide those projects with ASN1 functionality. */
 typedef struct {
     size_t offset;              /* Offset of this field in structure */
     byte type;                  /* The type of the member as defined in

wolfssl/wolfcrypt/ecc.h

@@ -641,7 +641,7 @@ int wc_ecc_export_point_der_ex(const int curve_idx, ecc_point* point, byte* out,
 WOLFSSL_API
 int wc_ecc_export_point_der(const int curve_idx, ecc_point* point,
                             byte* out, word32* outLen);
-WOLFSSL_API
+WOLFSSL_LOCAL
 int wc_ecc_export_point_der_compressed(const int curve_idx, ecc_point* point,
                                        byte* out, word32* outLen);
 #endif /* HAVE_ECC_KEY_EXPORT */
@@ -650,7 +650,7 @@ int wc_ecc_export_point_der_compressed(const int curve_idx, ecc_point* point,
 #ifdef HAVE_ECC_KEY_IMPORT
 WOLFSSL_API
 int wc_ecc_import_point_der_ex(byte* in, word32 inLen, const int curve_idx,
-                               ecc_point* point, char shortKeySize);
+                               ecc_point* point, int shortKeySize);
 WOLFSSL_API
 int wc_ecc_import_point_der(byte* in, word32 inLen, const int curve_idx,
                             ecc_point* point);