From e5352a7e570bcce20f202d790ab7b4d7aa7f6078 Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Fri, 3 Jul 2015 16:36:40 +0200 Subject: [PATCH] Fix parameter validation in ocsp [Problem] It's possible to pass invalid certificate chains to ocsp that will cause segfault. [Solution] Add argument check [Verification] Run ckm-tests --regexp=ocsp_check Change-Id: I267054f81780149a0512532a016c3f7caf30e900 --- src/manager/client-async/client-manager-async-impl.cpp | 2 ++ src/manager/client/client-manager-impl.cpp | 4 ++++ src/manager/service/ocsp-logic.cpp | 17 +++++++++++------ src/manager/service/ocsp.cpp | 4 ++-- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/manager/client-async/client-manager-async-impl.cpp b/src/manager/client-async/client-manager-async-impl.cpp index a04645e..2a37c24 100644 --- a/src/manager/client-async/client-manager-async-impl.cpp +++ b/src/manager/client-async/client-manager-async-impl.cpp @@ -248,6 +248,8 @@ void ManagerAsync::Impl::ocspCheck(const ObserverPtr& observer, try_catch_async([&] { RawBufferVector rawCertChain; for (auto &e: certificateChainVector) { + if(!e || e->empty()) + return observer->ReceivedError(CKM_API_ERROR_INPUT_PARAM); rawCertChain.push_back(e->getDER()); } diff --git a/src/manager/client/client-manager-impl.cpp b/src/manager/client/client-manager-impl.cpp index ca9d250..3bb1ef9 100644 --- a/src/manager/client/client-manager-impl.cpp +++ b/src/manager/client/client-manager-impl.cpp @@ -711,6 +711,10 @@ int ManagerImpl::ocspCheck(const CertificateShPtrVector &certChain, int &ocspSta RawBufferVector rawCertChain; for (auto &e: certChain) { + if (!e || e->empty()) { + LogError("Empty certificate"); + return CKM_API_ERROR_INPUT_PARAM; + } rawCertChain.push_back(e->getDER()); } diff --git a/src/manager/service/ocsp-logic.cpp b/src/manager/service/ocsp-logic.cpp index bd62d2c..af39e20 100644 --- a/src/manager/service/ocsp-logic.cpp +++ b/src/manager/service/ocsp-logic.cpp @@ -37,12 +37,17 @@ RawBuffer OCSPLogic::ocspCheck(int commandId, const RawBufferVector &rawChain) { int retCode = CKM_API_SUCCESS; int ocspStatus = CKM_API_OCSP_STATUS_INTERNAL_ERROR; - for (auto &e: rawChain) { - certChain.push_back(CertificateImpl(e, DataFormat::FORM_DER)); - if (certChain.rbegin()->empty()) { - LogDebug("Error in parsing certificates!"); - retCode = CKM_API_ERROR_INPUT_PARAM; - break; + if(rawChain.size() < 2) { + LogError("Certificate chain should contain at least 2 certificates"); + retCode = CKM_API_ERROR_INPUT_PARAM; + } else { + for (auto &e: rawChain) { + certChain.push_back(CertificateImpl(e, DataFormat::FORM_DER)); + if (certChain.rbegin()->empty()) { + LogDebug("Error in parsing certificates!"); + retCode = CKM_API_ERROR_INPUT_PARAM; + break; + } } } diff --git a/src/manager/service/ocsp.cpp b/src/manager/service/ocsp.cpp index 4f4477e..1dde8be 100644 --- a/src/manager/service/ocsp.cpp +++ b/src/manager/service/ocsp.cpp @@ -72,7 +72,7 @@ int OCSPModule::verify(const CertificateImplVector &certificateChain) { X509_STACK_PTR trustedCerts = create_x509_stack(); // skip first 2 certificates - for (auto it=certificateChain.cbegin()+2; it != certificateChain.cend(); it++) + for (auto it=certificateChain.cbegin()+2; it < certificateChain.cend(); it++) { if (it->empty()) { LogError("Error. Broken certificate chain."); @@ -81,7 +81,7 @@ int OCSPModule::verify(const CertificateImplVector &certificateChain) { sk_X509_push(trustedCerts.get(), it->getX509()); } - for (unsigned int i=0; i < certificateChain.size() -1; i++) {// except root certificate + for (int i=0; i < static_cast(certificateChain.size())-1; i++) {// except root certificate if (certificateChain[i].empty() || certificateChain[i+1].empty()) { LogError("Error. Broken certificate chain."); return CKM_API_OCSP_STATUS_INTERNAL_ERROR; -- 2.7.4