diff options
| author | Tobias Markmann <tm@ayena.de> | 2015-11-03 12:20:20 (GMT) |
|---|---|---|
| committer | Swift Review <review@swift.im> | 2015-11-06 10:50:52 (GMT) |
| commit | 7302ee18a137f6810ef1cc40a395f99d2f46a644 (patch) | |
| tree | 6360ffe3666118ad4e30628336c8705e240bd465 /Swiften | |
| parent | 8405fa16b738b6ef6a5920cd9d0f5735f8b62369 (diff) | |
| download | swift-7302ee18a137f6810ef1cc40a395f99d2f46a644.zip swift-7302ee18a137f6810ef1cc40a395f99d2f46a644.tar.bz2 | |
Fix potential memory leaks in Cocoa API usage
These errors were reported by Clang Analyzer.
Test-Information:
Verified that behavior is still as expected and Clang
Analyzer does not report the warnings anymore.
Change-Id: I149d75241f7680a6d2f2b6b710dd38d1ed81a209
Diffstat (limited to 'Swiften')
| -rw-r--r-- | Swiften/TLS/SecureTransport/SecureTransportCertificate.mm | 16 | ||||
| -rw-r--r-- | Swiften/TLS/SecureTransport/SecureTransportContext.mm | 8 |
2 files changed, 14 insertions, 10 deletions
diff --git a/Swiften/TLS/SecureTransport/SecureTransportCertificate.mm b/Swiften/TLS/SecureTransport/SecureTransportCertificate.mm index 3b4e00f..4270a6f 100644 --- a/Swiften/TLS/SecureTransport/SecureTransportCertificate.mm +++ b/Swiften/TLS/SecureTransport/SecureTransportCertificate.mm @@ -32,18 +32,19 @@ SecureTransportCertificate::SecureTransportCertificate(SecCertificateRef certifi CFRetain(certificate); certificateHandle_ = boost::shared_ptr<SecCertificate>(certificate, CFRelease); parse(); } SecureTransportCertificate::SecureTransportCertificate(const ByteArray& der) { CFDataRef derData = CFDataCreateWithBytesNoCopy(NULL, der.data(), static_cast<CFIndex>(der.size()), NULL); SecCertificateRef certificate = SecCertificateCreateWithData(NULL, derData); + CFRelease(derData); if (certificate) { certificateHandle_ = boost::shared_ptr<SecCertificate>(certificate, CFRelease); parse(); } } SecureTransportCertificate::~SecureTransportCertificate() { } @@ -51,36 +52,35 @@ SecureTransportCertificate::~SecureTransportCertificate() { #define NS2STDSTRING(a) (a == nil ? std::string() : std::string([a cStringUsingEncoding:NSUTF8StringEncoding])) void SecureTransportCertificate::parse() { assert(certificateHandle_); CFErrorRef error = NULL; // The SecCertificateCopyValues function is not part of the iOS Secure Transport API. CFDictionaryRef valueDict = SecCertificateCopyValues(certificateHandle_.get(), 0, &error); - if (error) { - CFRelease(error); - } - else { + if (valueDict) { // Handle subject. CFStringRef subject = SecCertificateCopySubjectSummary(certificateHandle_.get()); if (subject) { NSString* subjectStr = bridge_cast<NSString*>(subject); subjectName_ = NS2STDSTRING(subjectStr); CFRelease(subject); } // Handle a single Common Name. - CFStringRef commonName; + CFStringRef commonName = NULL; OSStatus error = SecCertificateCopyCommonName(certificateHandle_.get(), &commonName); - if (!error) { + if (!error && commonName) { NSString* commonNameStr = bridge_cast<NSString*>(commonName); commonNames_.push_back(NS2STDSTRING(commonNameStr)); + } + if (commonName) { CFRelease(commonName); } // Handle Subject Alternative Names NSDictionary* certDict = bridge_cast<NSDictionary*>(valueDict); NSDictionary* subjectAltNamesDict = certDict[@"2.5.29.17"][@"value"]; for (NSDictionary* entry in subjectAltNamesDict) { if ([entry[@"label"] isEqualToString:[NSString stringWithUTF8String:ID_ON_XMPPADDR_OID]]) { @@ -89,18 +89,22 @@ void SecureTransportCertificate::parse() { else if ([entry[@"label"] isEqualToString:[NSString stringWithUTF8String:ID_ON_DNSSRV_OID]]) { srvNames_.push_back(NS2STDSTRING(entry[@"value"])); } else if ([entry[@"label"] isEqualToString:@"DNS Name"]) { dnsNames_.push_back(NS2STDSTRING(entry[@"value"])); } } CFRelease(valueDict); } + + if (error) { + CFRelease(error); + } } std::string SecureTransportCertificate::getSubjectName() const { return subjectName_; } std::vector<std::string> SecureTransportCertificate::getCommonNames() const { return commonNames_; } diff --git a/Swiften/TLS/SecureTransport/SecureTransportContext.mm b/Swiften/TLS/SecureTransport/SecureTransportContext.mm index 7f44f7d..a702cde 100644 --- a/Swiften/TLS/SecureTransport/SecureTransportContext.mm +++ b/Swiften/TLS/SecureTransport/SecureTransportContext.mm @@ -32,19 +32,19 @@ T bridge_cast(S source) { return (__bridge T)(source); #pragma clang diagnostic pop } namespace Swift { namespace { -CFArrayRef getClientCertificateChainAsCFArrayRef(CertificateWithKey::ref key) { +CFArrayRef CreateClientCertificateChainAsCFArrayRef(CertificateWithKey::ref key) { boost::shared_ptr<PKCS12Certificate> pkcs12 = boost::dynamic_pointer_cast<PKCS12Certificate>(key); if (!key) { return NULL; } SafeByteArray safePassword = pkcs12->getPassword(); CFIndex passwordSize = 0; try { passwordSize = boost::numeric_cast<CFIndex>(safePassword.size()); @@ -58,31 +58,31 @@ CFArrayRef getClientCertificateChainAsCFArrayRef(CertificateWithKey::ref key) { CFStringRef password = CFStringCreateWithBytes(kCFAllocatorDefault, safePassword.data(), passwordSize, kCFStringEncodingUTF8, false); const void* keys[] = { kSecImportExportPassphrase }; const void* values[] = { password }; CFDictionaryRef options = CFDictionaryCreate(NULL, keys, values, 1, NULL, NULL); CFArrayRef items = NULL; CFDataRef pkcs12Data = bridge_cast<CFDataRef>([NSData dataWithBytes: static_cast<const void *>(pkcs12->getData().data()) length:pkcs12->getData().size()]); securityError = SecPKCS12Import(pkcs12Data, options, &items); + CFRelease(options); NSArray* nsItems = bridge_cast<NSArray*>(items); switch(securityError) { case errSecSuccess: break; case errSecAuthFailed: // Password did not work for decoding the certificate. case errSecDecode: // Other decoding error. default: CFRelease(certChain); CFRelease(items); - CFRelease(options); certChain = NULL; } if (certChain) { CFArrayAppendValue(certChain, nsItems[0][@"identity"]); for (CFIndex index = 0; index < CFArrayGetCount(bridge_cast<CFArrayRef>(nsItems[0][@"chain"])); index++) { CFArrayAppendValue(certChain, CFArrayGetValueAtIndex(bridge_cast<CFArrayRef>(nsItems[0][@"chain"]), index)); } @@ -144,19 +144,19 @@ std::string SecureTransportContext::stateToString(State state) { void SecureTransportContext::setState(State newState) { SWIFT_LOG(debug) << "Switch state from " << stateToString(state_) << " to " << stateToString(newState) << "." << std::endl; state_ = newState; } void SecureTransportContext::connect() { SWIFT_LOG_ASSERT(state_ == None, error) << "current state '" << stateToString(state_) << " invalid." << std::endl; if (clientCertificate_) { - CFArrayRef certs = getClientCertificateChainAsCFArrayRef(clientCertificate_); + CFArrayRef certs = CreateClientCertificateChainAsCFArrayRef(clientCertificate_); if (certs) { boost::shared_ptr<CFArray> certRefs(certs, CFRelease); OSStatus result = SSLSetCertificate(sslContext_.get(), certRefs.get()); if (result != noErr) { SWIFT_LOG(error) << "SSLSetCertificate failed with error " << result << "." << std::endl; } } } processHandshake(); @@ -268,19 +268,19 @@ void SecureTransportContext::verifyServerCertificate() { else { // proceed with handshake processHandshake(); } } #pragma clang diagnostic pop bool SecureTransportContext::setClientCertificate(CertificateWithKey::ref cert) { - CFArrayRef nativeClientChain = getClientCertificateChainAsCFArrayRef(cert); + CFArrayRef nativeClientChain = CreateClientCertificateChainAsCFArrayRef(cert); if (nativeClientChain) { clientCertificate_ = cert; CFRelease(nativeClientChain); return true; } else { return false; } } |
Swift