Fix SVACE defects

 * dereferencing null
 * memory leak
 * error return value unchecked

Change-Id: If87acb0817190955cc9c49d044a8b6003e7ac238
Signed-off-by: Kyungwook Tak <k.tak@samsung.com>

diff --git a/tests/pkcs12/new_test_cases.cpp b/tests/pkcs12/new_test_cases.cpp
index 7f2887e..12fe993 100644 (file)
--- a/tests/pkcs12/new_test_cases.cpp
+++ b/tests/pkcs12/new_test_cases.cpp
@@ -289,8 +289,8 @@ RUNNER_TEST(CERTSVC_PKCS12_1007_install_p12_file_to_individual_store)
        CREATE_INSTANCE
 
        CertSvcString Alias = wrapper_certsvc_string_new("P12-WifiUser");
-       CertSvcString Path = wrapper_certsvc_string_new("/usr/share/cert-svc/tests/wifiuser.p12");
-       CertSvcString Pass = wrapper_certsvc_string_new("wifi");
+       CertSvcString Path = wrapper_certsvc_string_new(TestData::UserP12WithPassPath.c_str());
+       CertSvcString Pass = wrapper_certsvc_string_new(TestData::UserP12Pass.c_str());
 
        result = certsvc_pkcs12_import_from_file_to_store(instance, WIFI_STORE, Path, Pass, Alias);
        RUNNER_ASSERT_MSG(result == CERTSVC_SUCCESS, "Importing p12 file to WIFI store failed.");

diff --git a/vcore/server/src/cert-server-main.c b/vcore/server/src/cert-server-main.c
index aa11454..9436ca2 100644 (file)
--- a/vcore/server/src/cert-server-main.c
+++ b/vcore/server/src/cert-server-main.c
@@ -302,6 +302,9 @@ Error_close_exit:
 
        if (client_sockfd >= 0) {
                result = send(client_sockfd, (char*)&send_data, sizeof(send_data), 0);
+               if (result <= 0)
+                       SLOGE("send failed :%d, errno %d try once", result, errno);
+
                close(client_sockfd);
        } else {
                SLOGE("cannot connect to client socket.");

diff --git a/vcore/vcore/Certificate.cpp b/vcore/vcore/Certificate.cpp
index ba27346..440020f 100644 (file)
--- a/vcore/vcore/Certificate.cpp
+++ b/vcore/vcore/Certificate.cpp
@@ -421,7 +421,10 @@ Certificate::AltNameSet Certificate::getAlternativeNameDNS() const
             X509_get_ext_d2i(m_x509,NID_subject_alt_name,NULL,NULL));
 
     while (sk_GENERAL_NAME_num(san) > 0) {
-        namePart = sk_GENERAL_NAME_pop(san);
+        if ((namePart = sk_GENERAL_NAME_pop(san)) == NULL)
+            VcoreThrowMsg(Certificate::Exception::OpensslInternalError,
+                          "openssl sk_GENERAL_NAME_pop err.");
+
         if (GEN_DNS == namePart->type) {
             char *temp = reinterpret_cast<char *>(ASN1_STRING_data(namePart->d.dNSName));
             if (!temp) {

diff --git a/vcore/vcore/Client.cpp b/vcore/vcore/Client.cpp
index e7c8c05..93aa8ed 100644 (file)
--- a/vcore/vcore/Client.cpp
+++ b/vcore/vcore/Client.cpp
@@ -26,14 +26,17 @@
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <errno.h>
+#include <unistd.h>
 #include <string.h>
 #include <stdlib.h>
-#include <unistd.h>
+#include <memory>
 
 #include <dpl/log/log.h>
 
 #include <vcore/Client.h>
 
+namespace {
+
 void initialize_res_data(VcoreResponseData *pData)
 {
        memset(pData->dataBlock, 0, VCORE_MAX_RECV_DATA_SIZE);
@@ -63,6 +66,43 @@ void initialize_req_data(VcoreRequestData *pData)
        pData->certType = INVALID_DATA;
 }
 
+CertSvcStoreCertList *createStoreListNode(VcoreCertResponseData *cert)
+{
+       CertSvcStoreCertList *node = NULL;
+
+       if (cert == NULL || cert->gname == NULL || cert->title == NULL)
+               return NULL;
+
+       node = (CertSvcStoreCertList *)malloc(sizeof(CertSvcStoreCertList));
+       if (node == NULL)
+               return NULL;
+
+       node->gname = strdup(cert->gname);
+       node->title = strdup(cert->title);
+       node->status = cert->status;
+       node->storeType = cert->storeType;
+       node->next = NULL;
+
+       if (node->gname == NULL || node->title == NULL) {
+               free(node->gname);
+               free(node->title);
+               free(node);
+               return NULL;
+       }
+
+       return node;
+}
+
+void destroyStoreList(CertSvcStoreCertList *list)
+{
+       while (list) {
+               CertSvcStoreCertList *next = list->next;
+               free(list);
+               list = next;
+       }
+}
+
+
 int _recv_fixed_lenghth(int sockfd, char *buff, int length)
 {
        int offset = 0;
@@ -156,9 +196,7 @@ VcoreRequestData* set_request_data(
        return pReqData;
 }
 
-
-VcoreResponseData cert_svc_client_comm(VcoreRequestData* pClientData) {
-
+VcoreResponseData cert_svc_client_comm(VcoreRequestData *pClientData) {
        int sockfd = 0;
        int clientLen = 0;
        int tempSockLen = 0;
@@ -268,6 +306,9 @@ Error_exit:
        return recvData;
 }
 
+} /* anonymous namespace */
+
+
 int vcore_client_install_certificate_to_store(
        CertStoreType storeType,
        const char *gname,
@@ -446,61 +487,46 @@ int vcore_client_delete_certificate_from_store(CertStoreType storeType, const ch
        return recvData.result;
 }
 
-int _vcore_client_get_certificate_list_from_store(VcoreRequestType reqType, CertStoreType storeType, int is_root_app, 
+int _vcore_client_get_certificate_list_from_store(VcoreRequestType reqType, CertStoreType storeType, int is_root_app,
                                                                           CertSvcStoreCertList **certList, size_t *length)
 {
-       VcoreRequestData* pSendData = NULL;
-       VcoreResponseData recvData;
-       CertSvcStoreCertList* curr = NULL;
-       CertSvcStoreCertList* prev = NULL;
-       VcoreCertResponseData* cert = NULL;
-       size_t tmplen = 0;
-       size_t i=0;
-       initialize_res_data(&recvData);
-
-
-       pSendData = set_request_data(reqType, storeType, is_root_app, NULL, NULL, NULL, NULL, NULL, 0, INVALID_DATA, DISABLED);
-       if (pSendData == NULL) {
+       std::unique_ptr<VcoreRequestData, void(*)(void *)> pSendData(set_request_data(
+                       reqType, storeType, is_root_app,
+                       NULL, NULL, NULL, NULL, NULL, 0, INVALID_DATA, DISABLED), free);
+       if (!pSendData) {
                LogError("Failed to set request data");
                return CERTSVC_WRONG_ARGUMENT;
        }
 
-       recvData = cert_svc_client_comm(pSendData);
+       VcoreResponseData recvData;
 
-       if(recvData.certCount > 0) {
-               for(i=0; i<recvData.certCount; i++) {
-                  cert = recvData.certList + i ;
-                  curr = (CertSvcStoreCertList*) malloc(sizeof(CertSvcStoreCertList));
-                  memset(curr, 0x00, sizeof(CertSvcStoreCertList));
-
-                  tmplen = strlen(cert->gname);
-                  curr->gname = (char*) malloc (sizeof(char) * (tmplen+ 1));
-                  memset(curr->gname, 0x00, tmplen + 1);
-                  memcpy(curr->gname, cert->gname, tmplen);
-
-                  tmplen = strlen(cert->title);
-                  curr->title = (char*) malloc (sizeof(char) * (tmplen+ 1));
-                  memset(curr->title, 0x00, tmplen + 1);
-                  memcpy(curr->title, cert->title, tmplen);
-
-                  curr->status = cert->status;
-                  curr->storeType = cert->storeType;
-
-                  if(prev == NULL) {
-                          *certList = curr;
-                  }else {
-                          prev->next = curr;
-                  }
-                  prev = curr;
+       initialize_res_data(&recvData);
+       recvData = cert_svc_client_comm(pSendData.get());
+
+       CertSvcStoreCertList *curr = NULL;
+       CertSvcStoreCertList *prev = NULL;
+       CertSvcStoreCertList *list = NULL;
+       for (size_t i = 0; i < recvData.certCount; i++) {
+               curr = createStoreListNode(recvData.certList + i);
+               if (curr == NULL) {
+                       destroyStoreList(list);
+                       free(recvData.certList);
+                       return CERTSVC_BAD_ALLOC;
                }
+
+               if (list == NULL)
+                       list = curr;
+               else
+                       prev->next = curr;
+               prev = curr;
        }
 
        *length = recvData.certCount;
+       *certList = list;
 
        LogDebug("get_certificate_list_from_store: result : " << recvData.result);
 
        free(recvData.certList);
-       free(pSendData);
 
        return recvData.result;
 }
@@ -552,7 +578,6 @@ int vcore_client_get_certificate_alias_from_store(CertStoreType storeType, const
 
 int vcore_client_load_certificates_from_store(CertStoreType storeType, const char *gname, char ***certs, size_t *ncerts)
 {
-       VcoreRequestData* pSendData = NULL;
        VcoreResponseData recvData;
        ResponseCertBlock* cert = NULL;
        size_t i = 0;
@@ -561,13 +586,16 @@ int vcore_client_load_certificates_from_store(CertStoreType storeType, const cha
 
        initialize_res_data(&recvData);
 
-       pSendData = set_request_data(CERTSVC_LOAD_CERTIFICATES, storeType, DISABLED, gname, NULL, NULL, NULL, NULL, 0, INVALID_DATA, DISABLED);
-       if (pSendData == NULL) {
+       std::unique_ptr<VcoreRequestData, void(*)(void *)> pSendData(set_request_data(
+                       CERTSVC_LOAD_CERTIFICATES, storeType, DISABLED, gname,
+                       NULL, NULL, NULL, NULL, 0, INVALID_DATA, DISABLED), free);
+
+       if (!pSendData) {
                LogError("Failed to set request data");
                return CERTSVC_WRONG_ARGUMENT;
        }
 
-       recvData = cert_svc_client_comm(pSendData);
+       recvData = cert_svc_client_comm(pSendData.get());
        if (recvData.result != CERTSVC_SUCCESS) {
                LogError("Failed to CERTSVC_LOAD_CERTIFICATES. server retcode : " << recvData.result);
                return recvData.result;
@@ -595,7 +623,6 @@ int vcore_client_load_certificates_from_store(CertStoreType storeType, const cha
        *ncerts = ncerts_out;
 
        free(recvData.certBlockList);
-       free(pSendData);
 
        return recvData.result;
 }

diff --git a/vcore/vcore/Client.h b/vcore/vcore/Client.h
index 2a33331..7a35789 100644 (file)
--- a/vcore/vcore/Client.h
+++ b/vcore/vcore/Client.h
@@ -103,7 +103,6 @@ int vcore_client_check_alias_exist_in_store(CertStoreType storeType, const char
 int vcore_client_install_certificate_to_store(CertStoreType storeType, const char *gname, const char *common_name, const char *private_key_gname, const char *associated_gname, const char *dataBlock, size_t dataBlockLen, CertType certType);
 int vcore_client_get_certificate_from_store(CertStoreType storeType, const char *gname, char **certData, size_t *certSize, CertType certType);
 int vcore_client_delete_certificate_from_store(CertStoreType storeType, const char *gname);
-VcoreResponseData cert_svc_client_comm(VcoreRequestData *client_data);
 int vcore_client_get_certificate_list_from_store(CertStoreType storeType, int is_root_app, CertSvcStoreCertList **certList, size_t *length);
 int vcore_client_get_root_certificate_list_from_store(CertStoreType storeType, CertSvcStoreCertList **certList, size_t *length);
 int vcore_client_get_end_user_certificate_list_from_store(CertStoreType storeType, CertSvcStoreCertList **certList, size_t *length);