Potential bug fixes detected by SVACE

use after free           : TADC_IF_VerifyCertChain (TADC_IF.cpp)
buffer overflow          : TADC_GetHashReqID       (TADC_Core.cpp)
checking condition error : DTappsGetCEK            (DTapps2Rights.cpp)

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

diff --git a/tadcore/TADCCore/TADC_Core.cpp b/tadcore/TADCCore/TADC_Core.cpp
index 86fbc6b..3204ebd 100644 (file)
--- a/tadcore/TADCCore/TADC_Core.cpp
+++ b/tadcore/TADCCore/TADC_Core.cpp
@@ -336,19 +336,15 @@ int TADC_MakeRequestRO(T_ROACQ_INFO *t_ROAcqInfo, unsigned char * outBuffer, siz
 
 int TADC_GetHashReqID(unsigned char * inBuffer, unsigned char *hashReqID)
 {
-       int i = 0, j = 0, nSize = 0;
-       char tmpbuf[512];
-       int length = 0;
-       int nResult = 0;
-
        //Check Param Buffer
        IF_TRUE_RETURN(inBuffer == NULL, TADC_PARAMETER_ERROR);
        IF_TRUE_RETURN(hashReqID == NULL, TADC_PARAMETER_ERROR);
-       nSize = TADC_IF_StrLen((char*)inBuffer);
+       int nSize = TADC_IF_StrLen((char*)inBuffer);
        IF_TRUE_RETURN(nSize <= 40 || nSize>RESP_MAXSIZE, TADC_PARAMETER_ERROR);
 
        //Check XML Result Code ( Success result='0' )
-       nResult = -1;
+       int nResult = -1;
+       int i = 0;
 
        for (i = 0 ; i < nSize ; i++)
        {
@@ -368,36 +364,28 @@ int TADC_GetHashReqID(unsigned char * inBuffer, unsigned char *hashReqID)
        }
        IF_TRUE_RETURN(((i == nSize) || (nResult < 0)), TADC_RESPONSEMESSAGE_ERROR);
 
-       //Init
-       TADC_IF_MemSet(tmpbuf, 0, sizeof(tmpbuf));
-
        //Get reqid
-       length = 0;
+       int length = 0;
+       char tmpbuf[512] = {0, };
 
        for (i = 0 ; i < nSize ; i++)
        {
                if (!TADC_IF_MemCmp(inBuffer + i, "reqid=", 6))
                {
                        i += 6;
-                       for (j = i ; j < nSize ; j++)
-                       {
-                               if (!TADC_IF_MemCmp(inBuffer + j, ";", 1))
-                               {
-                                       length = j - i;
-                                       TADC_IF_StrNCpy((char*)tmpbuf, (char*)(inBuffer + i), length);
-                                       tmpbuf[length] = 0;
-                                       break;
-                               }
-                       }
+                       length = 40;
+                       IF_TRUE_RETURN(i + length > nSize || inBuffer[i + length] != ';',
+                                       TADC_RESPONSEMESSAGE_ERROR);
+
+                       TADC_IF_StrNCpy(tmpbuf, (char*)(inBuffer + i), length);
+                       tmpbuf[length] = '\0';
+
                        break;
                }
        }
-       IF_TRUE_RETURN(length <= 0, TADC_RESPONSEMESSAGE_ERROR);
 
-       if ((length = TADC_IF_StrLen(tmpbuf)) != 40)
-       {
-               return -1;
-       }
+       // reqid not found
+       IF_TRUE_RETURN(length == 0, TADC_RESPONSEMESSAGE_ERROR);
 
        if ((nResult = HEX2BIN((char*)tmpbuf, hashReqID, &length)) < 0)
        {

diff --git a/tadcore/TADCInterface/TADC_IF.cpp b/tadcore/TADCInterface/TADC_IF.cpp
index 8411132..ef85b20 100644 (file)
--- a/tadcore/TADCInterface/TADC_IF.cpp
+++ b/tadcore/TADCInterface/TADC_IF.cpp
@@ -36,6 +36,8 @@
 
 #include <dirent.h>
 
+#include <memory>
+
 #include "DUIDGenerator.h"
 
 
@@ -395,68 +397,52 @@ error:
 int TADC_IF_VerifyCertChain(unsigned char* rica, int ricaLen,
                                                        unsigned char* cert, int certLen)
 {
-       X509_STORE_CTX* pstStoreCtx = NULL;
-       X509_STORE* pstStore = NULL;
-       STACK_OF(X509)* untrustedCerts = NULL;
-
-       X509* pstX509 = NULL;
-
-       int iRet = 0;
-       int iErrCode = 0;
-
-       //must call this function.
        OpenSSL_add_all_algorithms();
 
-       pstStore = X509_STORE_new();
-       if(pstStore == NULL)
-       {
-               iRet = -1;
-               goto error;
-       }
+       X509_STORE *pstStore = X509_STORE_new();
+       if (pstStore == NULL)
+               return -1;
 
-       untrustedCerts = sk_X509_new_null();
-       if(untrustedCerts == NULL)
-       {
-               iRet = -1;
-               goto error;
-       }
+       std::unique_ptr<X509_STORE, void(*)(X509_STORE *)>
+               _scoped_x509_store(pstStore, X509_STORE_free);
+
+       STACK_OF(X509) *untrustedCerts = sk_X509_new_null();
+       if (untrustedCerts == NULL)
+               return -1;
 
+       std::unique_ptr<STACK_OF(X509), std::function<void(STACK_OF(X509) *)>>
+               _scoped_x509_stack(untrustedCerts, [](STACK_OF(X509) *s) { sk_X509_free(s); });
 
        //Add RICA Cert to certchain
-       if ((iRet = AddCertUntrustedCerts(untrustedCerts, rica, ricaLen)) != 0)
-       {
+       if (AddCertUntrustedCerts(untrustedCerts, rica, ricaLen) != 0) {
                DRM_TAPPS_EXCEPTION("TADC_IF_VerifyCertChain Error : Add RICA Cert to certchain!");
-               iRet = -1;
-               goto error;
+               return -1;
        }
 
        //Add Root CA Cert
-       if ((iRet = AddCertSTOREFromDir(pstStore, RO_ISSUER_ROOT_CERTS_DIR)) != 0)
-       {
+       if (AddCertSTOREFromDir(pstStore, RO_ISSUER_ROOT_CERTS_DIR) != 0) {
                DRM_TAPPS_EXCEPTION("TADC_IF_VerifyCertChain Error : Add Root CA Cert!");
-               iRet = -1;
-               goto error;
+               return -1;
        }
 
        //Get Cert
-       pstX509 = d2i_X509(NULL, (const unsigned char **)&cert, certLen);
+       X509 *pstX509 = d2i_X509(NULL, (const unsigned char **)&cert, certLen);
 
-       if (pstX509 == NULL)
-       {
+       if (pstX509 == NULL) {
                DRM_TAPPS_EXCEPTION("TADC_IF_VerifyCertChain Error : Get Cert d2i_X509 error!");
-               iRet = -1;
-               goto error;
+               return -1;
        }
 
        X509_STORE_set_flags(pstStore, X509_V_FLAG_CB_ISSUER_CHECK);
-       pstStoreCtx = X509_STORE_CTX_new();
-       if (pstStoreCtx == NULL)
-       {
+       X509_STORE_CTX *pstStoreCtx = X509_STORE_CTX_new();
+       if (pstStoreCtx == NULL) {
                DRM_TAPPS_EXCEPTION("TADC_IF_VerifyCertChain Error : 509_STORE_CTX_new error!");
-               iRet = -1;
-               goto error;
+               return -1;
        }
 
+       std::unique_ptr<X509_STORE_CTX, void(*)(X509_STORE_CTX *)>
+               _scoped_x509_store_ctx(pstStoreCtx, X509_STORE_CTX_free);
+
        //init
        X509_STORE_CTX_init(pstStoreCtx, pstStore, pstX509, untrustedCerts);
 
@@ -464,31 +450,16 @@ int TADC_IF_VerifyCertChain(unsigned char* rica, int ricaLen,
        X509_STORE_CTX_set_flags(pstStoreCtx, X509_V_FLAG_CB_ISSUER_CHECK);
 
        //verify
-       iRet = X509_verify_cert(pstStoreCtx);
-
-       //free
-error:
-       if (pstStore != NULL)
-               X509_STORE_free(pstStore);
-       if (pstStoreCtx != NULL)
-               X509_STORE_CTX_free(pstStoreCtx);
-       if (untrustedCerts != NULL)
-               sk_X509_free(untrustedCerts);
-
-       if (iRet == 1)
-       {
-               DRM_TAPPS_LOG("TADC_IF_VerifyCertChain Success! \n");
+       switch (X509_verify_cert(pstStoreCtx)) {
+       case 1:
+               DRM_TAPPS_LOG("TADC_IF_VerifyCertChain Success!");
                return 0;
-       }
-       else if (iRet == 0)
-       {
-               iErrCode = X509_STORE_CTX_get_error(pstStoreCtx);
-               DRM_TAPPS_EXCEPTION("TADC_IF_VerifyCertChain Error : %s \n", X509_verify_cert_error_string(iErrCode));
+       case 0:
+               DRM_TAPPS_EXCEPTION("TADC_IF_VerifyCertChain Failed: %s",
+                               X509_verify_cert_error_string(X509_STORE_CTX_get_error(pstStoreCtx)));
                return -1;
-       }
-       else
-       {
-               DRM_TAPPS_EXCEPTION("TADC_IF_VerifyCertChain Error : 509_verify_cert error! \n");
+       default:
+               DRM_TAPPS_EXCEPTION("TADC_IF_VerifyCertChain Error: X509_verify_cert error!");
                return -1;
        }
 }

diff --git a/tappsd/src/rights/DTapps2Rights.cpp b/tappsd/src/rights/DTapps2Rights.cpp
index 7b595f3..9b81050 100644 (file)
--- a/tappsd/src/rights/DTapps2Rights.cpp
+++ b/tappsd/src/rights/DTapps2Rights.cpp
@@ -879,7 +879,7 @@ BOOL DTappsGetCEK(const char* szCid, T_RO* t_RO)
 
                hash_buf_enclen = TAPPS_STRLEN(row.cek_hash);
                hash_buf_declen = hash_buf_enclen / 4 * 3;
-               if (db_buf_enclen <= 0)
+               if (hash_buf_enclen <= 0)
                {
                        DRM_TAPPS_EXCEPTION("There is no CEK_hash buffer in the DB. cid = %s", szCid);
                        check_valid = TADC_LICENSE_UNKNOWN_ERROR;