From ba00d89b99fb76f4123c27589a8dcc842ac8c1d1 Mon Sep 17 00:00:00 2001 From: Maarten Billemont Date: Tue, 19 Jul 2016 12:00:19 -0400 Subject: [PATCH] Fix some potential crashes and memory leaks. --- MasterPassword/C/mpw-util.c | 6 +- MasterPassword/ObjC/MPAlgorithmV0.m | 16 ++-- MasterPassword/ObjC/MPAppDelegate_Key.m | 4 +- .../ObjC/iOS/MasterPassword-Prefix.pch | 80 ++++--------------- 4 files changed, 31 insertions(+), 75 deletions(-) diff --git a/MasterPassword/C/mpw-util.c b/MasterPassword/C/mpw-util.c index bf70d2c6..eda606d4 100644 --- a/MasterPassword/C/mpw-util.c +++ b/MasterPassword/C/mpw-util.c @@ -54,8 +54,10 @@ void mpw_pushInt(uint8_t **const buffer, size_t *const bufferSize, const uint32_ void mpw_free(const void *buffer, const size_t bufferSize) { - memset( (void *)buffer, 0, bufferSize ); - free( (void *)buffer ); + if (buffer) { + memset( (void *)buffer, 0, bufferSize ); + free( (void *)buffer ); + } } void mpw_freeString(const char *string) { diff --git a/MasterPassword/ObjC/MPAlgorithmV0.m b/MasterPassword/ObjC/MPAlgorithmV0.m index bb9dc247..5843a690 100644 --- a/MasterPassword/ObjC/MPAlgorithmV0.m +++ b/MasterPassword/ObjC/MPAlgorithmV0.m @@ -140,10 +140,12 @@ NSOperationQueue *_mpwQueue = nil; [self mpw_perform:^{ NSDate *start = [NSDate date]; uint8_t const *masterKeyBytes = mpw_masterKeyForUser( fullName.UTF8String, masterPassword.UTF8String, [self version] ); - keyData = [NSData dataWithBytes:masterKeyBytes length:MP_dkLen]; - trc( @"User: %@, password: %@ derives to key ID: %@ (took %0.2fs)", // - fullName, masterPassword, [self keyIDForKeyData:keyData], -[start timeIntervalSinceNow] ); - mpw_free( masterKeyBytes, MP_dkLen ); + if (masterKeyBytes) { + keyData = [NSData dataWithBytes:masterKeyBytes length:MP_dkLen]; + trc( @"User: %@, password: %@ derives to key ID: %@ (took %0.2fs)", // + fullName, masterPassword, [self keyIDForKeyData:keyData], -[start timeIntervalSinceNow] ); + mpw_free( masterKeyBytes, MP_dkLen ); + } }]; return keyData; @@ -358,8 +360,10 @@ NSOperationQueue *_mpwQueue = nil; [self mpw_perform:^{ char const *contentBytes = mpw_passwordForSite( [key keyDataForAlgorithm:self].bytes, name.UTF8String, type, (uint32_t)counter, variant, context.UTF8String, [self version] ); - content = [NSString stringWithCString:contentBytes encoding:NSUTF8StringEncoding]; - mpw_freeString( contentBytes ); + if (contentBytes) { + content = [NSString stringWithCString:contentBytes encoding:NSUTF8StringEncoding]; + mpw_freeString( contentBytes ); + } }]; return content; diff --git a/MasterPassword/ObjC/MPAppDelegate_Key.m b/MasterPassword/ObjC/MPAppDelegate_Key.m index f681f65d..9fd0af6f 100644 --- a/MasterPassword/ObjC/MPAppDelegate_Key.m +++ b/MasterPassword/ObjC/MPAppDelegate_Key.m @@ -26,7 +26,7 @@ static NSDictionary *createKeyQuery(MPUserEntity *user, BOOL newItem, MPKeyOrigi *keyOrigin = MPKeyOriginKeyChainBiometric; CFErrorRef acError = NULL; - SecAccessControlRef accessControl = SecAccessControlCreateWithFlags( kCFAllocatorDefault, + id accessControl = (__bridge_transfer id)SecAccessControlCreateWithFlags( kCFAllocatorDefault, kSecAttrAccessibleWhenPasscodeSetThisDeviceOnly, kSecAccessControlTouchIDCurrentSet, &acError ); if (!accessControl || acError) err( @"Could not use TouchID on this device: %@", acError ); @@ -36,7 +36,7 @@ static NSDictionary *createKeyQuery(MPUserEntity *user, BOOL newItem, MPKeyOrigi attributes:@{ (__bridge id)kSecAttrService : @"Saved Master Password", (__bridge id)kSecAttrAccount : user.name?: @"", - (__bridge id)kSecAttrAccessControl : (__bridge id)accessControl, + (__bridge id)kSecAttrAccessControl : accessControl, (__bridge id)kSecUseAuthenticationUI : (__bridge id)kSecUseAuthenticationUIAllow, (__bridge id)kSecUseOperationPrompt : strf( @"Access %@'s master password.", user.name ), diff --git a/MasterPassword/ObjC/iOS/MasterPassword-Prefix.pch b/MasterPassword/ObjC/iOS/MasterPassword-Prefix.pch index f2273a8f..e7a4385d 100644 --- a/MasterPassword/ObjC/iOS/MasterPassword-Prefix.pch +++ b/MasterPassword/ObjC/iOS/MasterPassword-Prefix.pch @@ -31,76 +31,26 @@ #import #import -#define trc(format, ...) \ +#define log(level, format, ...) \ do { \ + void (*_sendMsg)(id, SEL, CFStringRef, NSInteger, CFStringRef, NSUInteger, CFStringRef) = (void *)objc_msgSend; \ char *_msg = NULL; \ asprintf( &_msg, format, ##__VA_ARGS__ ); \ - void (*_sendMsg)(id, SEL, CFStringRef, NSInteger, CFStringRef, NSUInteger, CFStringRef) = (void *)objc_msgSend; \ + CFStringRef fileStr = CFStringCreateWithCString( NULL, basename( (char *)__FILE__ ), kCFStringEncodingUTF8 ); \ + CFStringRef funcStr = CFStringCreateWithCString( NULL, __FUNCTION__, kCFStringEncodingUTF8 ); \ + CFStringRef msgStr = CFStringCreateWithCString( NULL, _msg, kCFStringEncodingUTF8 ); \ _sendMsg( objc_msgSend( (id)objc_getClass( "PearlLogger" ), sel_getUid( "get" ) ), \ - sel_getUid( "inFile:atLine:fromFunction:withLevel:text:" ), \ - CFStringCreateWithCString( NULL, basename( (char *)__FILE__ ), kCFStringEncodingUTF8 ), __LINE__, \ - CFStringCreateWithCString( NULL, __FUNCTION__, kCFStringEncodingUTF8 ), 0, \ - CFStringCreateWithCString( NULL, _msg, kCFStringEncodingUTF8 ) ); \ + sel_getUid( "inFile:atLine:fromFunction:withLevel:text:" ), fileStr, __LINE__, funcStr, level, msgStr ); \ + CFRelease( fileStr ); \ + CFRelease( funcStr ); \ + CFRelease( msgStr ); \ } while (0) -#define dbg(format, ...) \ - do { \ - char *_msg = NULL; \ - asprintf( &_msg, format, ##__VA_ARGS__ ); \ - void (*_sendMsg)(id, SEL, CFStringRef, NSInteger, CFStringRef, NSUInteger, CFStringRef) = (void *)objc_msgSend; \ - _sendMsg( objc_msgSend( (id)objc_getClass( "PearlLogger" ), sel_getUid( "get" ) ), \ - sel_getUid( "inFile:atLine:fromFunction:withLevel:text:" ), \ - CFStringCreateWithCString( NULL, basename( (char *)__FILE__ ), kCFStringEncodingUTF8 ), __LINE__, \ - CFStringCreateWithCString( NULL, __FUNCTION__, kCFStringEncodingUTF8 ), 1, \ - CFStringCreateWithCString( NULL, _msg, kCFStringEncodingUTF8 ) ); \ - } while (0) - -#define inf(format, ...) \ - do { \ - char *_msg = NULL; \ - asprintf( &_msg, format, ##__VA_ARGS__ ); \ - void (*_sendMsg)(id, SEL, CFStringRef, NSInteger, CFStringRef, NSUInteger, CFStringRef) = (void *)objc_msgSend; \ - _sendMsg( objc_msgSend( (id)objc_getClass( "PearlLogger" ), sel_getUid( "get" ) ), \ - sel_getUid( "inFile:atLine:fromFunction:withLevel:text:" ), \ - CFStringCreateWithCString( NULL, basename( (char *)__FILE__ ), kCFStringEncodingUTF8 ), __LINE__, \ - CFStringCreateWithCString( NULL, __FUNCTION__, kCFStringEncodingUTF8 ), 2, \ - CFStringCreateWithCString( NULL, _msg, kCFStringEncodingUTF8 ) ); \ - } while (0) - -#define wrn(format, ...) \ - do { \ - char *_msg = NULL; \ - asprintf( &_msg, format, ##__VA_ARGS__ ); \ - void (*_sendMsg)(id, SEL, CFStringRef, NSInteger, CFStringRef, NSUInteger, CFStringRef) = (void *)objc_msgSend; \ - _sendMsg( objc_msgSend( (id)objc_getClass( "PearlLogger" ), sel_getUid( "get" ) ), \ - sel_getUid( "inFile:atLine:fromFunction:withLevel:text:" ), \ - CFStringCreateWithCString( NULL, basename( (char *)__FILE__ ), kCFStringEncodingUTF8 ), __LINE__, \ - CFStringCreateWithCString( NULL, __FUNCTION__, kCFStringEncodingUTF8 ), 3, \ - CFStringCreateWithCString( NULL, _msg, kCFStringEncodingUTF8 ) ); \ - } while (0) - -#define err(format, ...) \ - do { \ - char *_msg = NULL; \ - asprintf( &_msg, format, ##__VA_ARGS__ ); \ - void (*_sendMsg)(id, SEL, CFStringRef, NSInteger, CFStringRef, NSUInteger, CFStringRef) = (void *)objc_msgSend; \ - _sendMsg( objc_msgSend( (id)objc_getClass( "PearlLogger" ), sel_getUid( "get" ) ), \ - sel_getUid( "inFile:atLine:fromFunction:withLevel:text:" ), \ - CFStringCreateWithCString( NULL, basename( (char *)__FILE__ ), kCFStringEncodingUTF8 ), __LINE__, \ - CFStringCreateWithCString( NULL, __FUNCTION__, kCFStringEncodingUTF8 ), 4, \ - CFStringCreateWithCString( NULL, _msg, kCFStringEncodingUTF8 ) ); \ - } while (0) - -#define ftl(format, ...) \ - do { \ - char *_msg = NULL; \ - asprintf( &_msg, format, ##__VA_ARGS__ ); \ - void (*_sendMsg)(id, SEL, CFStringRef, NSInteger, CFStringRef, NSUInteger, CFStringRef) = (void *)objc_msgSend; \ - _sendMsg( objc_msgSend( (id)objc_getClass( "PearlLogger" ), sel_getUid( "get" ) ), \ - sel_getUid( "inFile:atLine:fromFunction:withLevel:text:" ), \ - CFStringCreateWithCString( NULL, basename( (char *)__FILE__ ), kCFStringEncodingUTF8 ), __LINE__, \ - CFStringCreateWithCString( NULL, __FUNCTION__, kCFStringEncodingUTF8 ), 5, \ - CFStringCreateWithCString( NULL, _msg, kCFStringEncodingUTF8 ) ); \ - } while (0) +#define trc(format, ...) log( 0, format, ##__VA_ARGS__ ); +#define dbg(format, ...) log( 1, format, ##__VA_ARGS__ ); +#define inf(format, ...) log( 2, format, ##__VA_ARGS__ ); +#define wrn(format, ...) log( 3, format, ##__VA_ARGS__ ); +#define err(format, ...) log( 4, format, ##__VA_ARGS__ ); +#define ftl(format, ...) log( 5, format, ##__VA_ARGS__ ); #endif