From bfb083f50bdd18b6e7f4a63ca21cf9d0b9be3264 Mon Sep 17 00:00:00 2001 From: Kai Engert Date: Tue, 17 Apr 2012 01:36:46 +0200 Subject: [PATCH] Bug 633471, crash if I delete a PKCS#11 certificate when smart card is logged out. Patch contributed by timeless and Ludovic Rousseau, plus my cleanup. r=kaie --- security/manager/ssl/src/nsCertTree.cpp | 120 +++++++++++++----------- 1 file changed, 65 insertions(+), 55 deletions(-) diff --git a/security/manager/ssl/src/nsCertTree.cpp b/security/manager/ssl/src/nsCertTree.cpp index 5498afd2ff9a..d662c3b378f7 100644 --- a/security/manager/ssl/src/nsCertTree.cpp +++ b/security/manager/ssl/src/nsCertTree.cpp @@ -264,15 +264,17 @@ nsCertTree::CountOrganizations() certCount = mDispInfo.Length(); if (certCount == 0) return 0; nsCOMPtr orgCert = nsnull; - if (mDispInfo.ElementAt(0)->mAddonInfo) { - orgCert = mDispInfo.ElementAt(0)->mAddonInfo->mCert; + nsCertAddonInfo *addonInfo = mDispInfo.ElementAt(0)->mAddonInfo; + if (addonInfo) { + orgCert = addonInfo->mCert; } nsCOMPtr nextCert = nsnull; PRInt32 orgCount = 1; for (i=1; imAddonInfo) { - nextCert = mDispInfo.ElementAt(i)->mAddonInfo->mCert; + addonInfo = mDispInfo.SafeElementAt(i, NULL)->mAddonInfo; + if (addonInfo) { + nextCert = addonInfo->mCert; } // XXX we assume issuer org is always criterion 1 if (CmpBy(&mCompareCache, orgCert, nextCert, sort_IssuerOrg, sort_None, sort_None) != 0) { @@ -342,7 +344,7 @@ nsCertTree::GetDispInfoAtIndex(PRInt32 index, PRInt32 certIndex = cIndex + index - idx; if (outAbsoluteCertOffset) *outAbsoluteCertOffset = certIndex; - nsRefPtr certdi = mDispInfo.ElementAt(certIndex); + nsRefPtr certdi = mDispInfo.SafeElementAt(certIndex, NULL); if (certdi) { nsCertTreeDispInfo *raw = certdi.get(); NS_IF_ADDREF(raw); @@ -619,9 +621,9 @@ nsCertTree::GetCertsByTypeFromCertList(CERTCertList *aCertList, int InsertPosition = 0; for (; InsertPosition < count; ++InsertPosition) { nsCOMPtr cert = nsnull; - nsRefPtr elem = mDispInfo.ElementAt(InsertPosition); - if (elem->mAddonInfo) { - cert = mDispInfo.ElementAt(InsertPosition)->mAddonInfo->mCert; + nsRefPtr elem = mDispInfo.SafeElementAt(InsertPosition, NULL); + if (elem && elem->mAddonInfo) { + cert = elem->mAddonInfo->mCert; } if ((*aCertCmpFn)(aCertCmpFnArg, pipCert, cert) < 0) { break; @@ -752,8 +754,9 @@ nsCertTree::UpdateUIContents() if (count) { PRUint32 j = 0; nsCOMPtr orgCert = nsnull; - if (mDispInfo.ElementAt(j)->mAddonInfo) { - orgCert = mDispInfo.ElementAt(j)->mAddonInfo->mCert; + nsCertAddonInfo *addonInfo = mDispInfo.ElementAt(j)->mAddonInfo; + if (addonInfo) { + orgCert = addonInfo->mCert; } for (PRInt32 i=0; i= count) break; nsCOMPtr nextCert = nsnull; - if (mDispInfo.ElementAt(j)->mAddonInfo) { - nextCert = mDispInfo.ElementAt(j)->mAddonInfo->mCert; + nsCertAddonInfo *addonInfo = mDispInfo.SafeElementAt(j, NULL)->mAddonInfo; + if (addonInfo) { + nextCert = addonInfo->mCert; } while (0 == CmpBy(&mCompareCache, orgCert, nextCert, sort_IssuerOrg, sort_None, sort_None)) { mTreeArray[i].numChildren++; if (++j >= count) break; nextCert = nsnull; - if (mDispInfo.ElementAt(j)->mAddonInfo) { - nextCert = mDispInfo.ElementAt(j)->mAddonInfo->mCert; + addonInfo = mDispInfo.SafeElementAt(j, NULL)->mAddonInfo; + if (addonInfo) { + nextCert = addonInfo->mCert; } } orgCert = nextCert; @@ -818,52 +823,57 @@ nsCertTree::DeleteEntryObject(PRUint32 index) if (index < idx + nc) { // cert is within range of this thread PRInt32 certIndex = cIndex + index - idx; - nsRefPtr certdi = mDispInfo.ElementAt(certIndex); - nsCOMPtr cert = nsnull; - if (certdi->mAddonInfo) { - cert = certdi->mAddonInfo->mCert; - } bool canRemoveEntry = false; - - if (certdi->mTypeOfEntry == nsCertTreeDispInfo::host_port_override) { - mOverrideService->ClearValidityOverride(certdi->mAsciiHost, certdi->mPort); + nsRefPtr certdi = mDispInfo.SafeElementAt(certIndex, NULL); + + // We will remove the element from the visual tree. + // Only if we have a certdi, then we can check for additional actions. + nsCOMPtr cert = nsnull; + if (certdi) { if (certdi->mAddonInfo) { - certdi->mAddonInfo->mUsageCount--; - if (certdi->mAddonInfo->mUsageCount == 0) { - // The certificate stored in the database is no longer - // referenced by any other object displayed. - // That means we no longer need to keep it around - // and really can remove it. - canRemoveEntry = true; - } - } - } - else { - if (certdi->mAddonInfo->mUsageCount > 1) { - // user is trying to delete a perm trusted cert, - // although there are still overrides stored, - // so, we keep the cert, but remove the trust - - CERTCertificate *nsscert = nsnull; - CERTCertificateCleaner nsscertCleaner(nsscert); - - nsCOMPtr cert2 = do_QueryInterface(cert); - if (cert2) { - nsscert = cert2->GetCert(); - } - - if (nsscert) { - CERTCertTrust trust; - memset((void*)&trust, 0, sizeof(trust)); - - SECStatus srv = CERT_DecodeTrustString(&trust, ""); // no override - if (srv == SECSuccess) { - CERT_ChangeCertTrust(CERT_GetDefaultCertDB(), nsscert, &trust); + cert = certdi->mAddonInfo->mCert; + } + nsCertAddonInfo *addonInfo = certdi->mAddonInfo ? certdi->mAddonInfo : nsnull; + if (certdi->mTypeOfEntry == nsCertTreeDispInfo::host_port_override) { + mOverrideService->ClearValidityOverride(certdi->mAsciiHost, certdi->mPort); + if (addonInfo) { + addonInfo->mUsageCount--; + if (addonInfo->mUsageCount == 0) { + // The certificate stored in the database is no longer + // referenced by any other object displayed. + // That means we no longer need to keep it around + // and really can remove it. + canRemoveEntry = true; } - } + } } else { - canRemoveEntry = true; + if (addonInfo && addonInfo->mUsageCount > 1) { + // user is trying to delete a perm trusted cert, + // although there are still overrides stored, + // so, we keep the cert, but remove the trust + + CERTCertificate *nsscert = nsnull; + CERTCertificateCleaner nsscertCleaner(nsscert); + + nsCOMPtr cert2 = do_QueryInterface(cert); + if (cert2) { + nsscert = cert2->GetCert(); + } + + if (nsscert) { + CERTCertTrust trust; + memset((void*)&trust, 0, sizeof(trust)); + + SECStatus srv = CERT_DecodeTrustString(&trust, ""); // no override + if (srv == SECSuccess) { + CERT_ChangeCertTrust(CERT_GetDefaultCertDB(), nsscert, &trust); + } + } + } + else { + canRemoveEntry = true; + } } }