From 34a05cdb8527bc295ab26094ca04f2b644761655 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 6 Sep 2018 13:23:57 -0400 Subject: [PATCH 1/3] Refine image validation. --- .../attachments/SignalAttachment.swift | 15 ++-- .../Messages/Attachments/OWSMediaUtils.swift | 66 +++++++++++++++-- SignalServiceKit/src/Util/NSData+Image.m | 74 ++++++++++++++++--- SignalServiceKit/src/Util/UIImage+OWS.m | 6 +- 4 files changed, 131 insertions(+), 30 deletions(-) diff --git a/SignalMessaging/attachments/SignalAttachment.swift b/SignalMessaging/attachments/SignalAttachment.swift index 402c320c1..4e930c577 100644 --- a/SignalMessaging/attachments/SignalAttachment.swift +++ b/SignalMessaging/attachments/SignalAttachment.swift @@ -180,16 +180,11 @@ public class SignalAttachment: NSObject { // MARK: Constants - /** - * Media Size constraints from Signal-Android - * - * https://github.com/signalapp/Signal-Android/blob/master/src/org/thoughtcrime/securesms/mms/PushMediaConstraints.java - */ - static let kMaxFileSizeAnimatedImage = UInt(25 * 1024 * 1024) - static let kMaxFileSizeImage = UInt(6 * 1024 * 1024) - static let kMaxFileSizeVideo = UInt(100 * 1024 * 1024) - static let kMaxFileSizeAudio = UInt(100 * 1024 * 1024) - static let kMaxFileSizeGeneric = UInt(100 * 1024 * 1024) + static let kMaxFileSizeAnimatedImage = OWSMediaUtils.kMaxFileSizeAnimatedImage + static let kMaxFileSizeImage = OWSMediaUtils.kMaxFileSizeImage + static let kMaxFileSizeVideo = OWSMediaUtils.kMaxFileSizeVideo + static let kMaxFileSizeAudio = OWSMediaUtils.kMaxFileSizeAudio + static let kMaxFileSizeGeneric = OWSMediaUtils.kMaxFileSizeGeneric // MARK: Constructor diff --git a/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift b/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift index 4f78788c0..6abc6d0ef 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift +++ b/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift @@ -16,13 +16,14 @@ public enum OWSMediaError: Error { } @objc public class func thumbnail(forImageAtPath path: String, maxDimension: CGFloat) throws -> UIImage { + Logger.verbose("thumbnailing image: \(path)") + guard FileManager.default.fileExists(atPath: path) else { throw OWSMediaError.failure(description: "Media file missing.") } guard NSData.ows_isValidImage(atPath: path) else { throw OWSMediaError.failure(description: "Invalid image.") } - guard let originalImage = UIImage(contentsOfFile: path) else { throw OWSMediaError.failure(description: "Could not load original image.") } @@ -33,11 +34,13 @@ public enum OWSMediaError: Error { } @objc public class func thumbnail(forVideoAtPath path: String, maxDimension: CGFloat) throws -> UIImage { - let maxSize = CGSize(width: maxDimension, height: maxDimension) + Logger.verbose("thumbnailing video: \(path)") - guard FileManager.default.fileExists(atPath: path) else { - throw OWSMediaError.failure(description: "Media file missing.") + guard isVideoOfValidContentTypeAndSize(path: path) else { + throw OWSMediaError.failure(description: "Media file has missing or invalid length.") } + + let maxSize = CGSize(width: maxDimension, height: maxDimension) let url = URL(fileURLWithPath: path) let asset = AVURLAsset(url: url, options: nil) guard isValidVideo(asset: asset) else { @@ -54,15 +57,38 @@ public enum OWSMediaError: Error { } @objc public class func isValidVideo(path: String) -> Bool { - guard FileManager.default.fileExists(atPath: path) else { - Logger.error("Media file missing.") + guard isVideoOfValidContentTypeAndSize(path: path) else { + Logger.error("Media file has missing or invalid length.") return false } + let url = URL(fileURLWithPath: path) let asset = AVURLAsset(url: url, options: nil) return isValidVideo(asset: asset) } + private class func isVideoOfValidContentTypeAndSize(path: String) -> Bool { + guard FileManager.default.fileExists(atPath: path) else { + Logger.error("Media file missing.") + return false + } + let fileExtension = URL(fileURLWithPath: path).pathExtension + guard let contentType = MIMETypeUtil.mimeType(forFileExtension: fileExtension) else { + Logger.error("Media file has unknown content type.") + return false + } + guard MIMETypeUtil.isSupportedVideoMIMEType(contentType) else { + Logger.error("Media file has invalid content type.") + return false + } + + guard let fileSize = OWSFileSystem.fileSize(ofPath: path) else { + Logger.error("Media file has unknown length.") + return false + } + return fileSize.uintValue <= kMaxFileSizeVideo + } + private class func isValidVideo(asset: AVURLAsset) -> Bool { var maxTrackSize = CGSize.zero for track: AVAssetTrack in asset.tracks(withMediaType: .video) { @@ -74,11 +100,35 @@ public enum OWSMediaError: Error { Logger.error("Invalid video size: \(maxTrackSize)") return false } - let kMaxValidSize: CGFloat = 3 * 1024.0 - if maxTrackSize.width > kMaxValidSize || maxTrackSize.height > kMaxValidSize { + if maxTrackSize.width > kMaxVideoDimensions || maxTrackSize.height > kMaxVideoDimensions { Logger.error("Invalid video dimensions: \(maxTrackSize)") return false } return true } + + // MARK: Constants + + /** + * Media Size constraints from Signal-Android + * + * https://github.com/signalapp/Signal-Android/blob/master/src/org/thoughtcrime/securesms/mms/PushMediaConstraints.java + */ + @objc + public static let kMaxFileSizeAnimatedImage = UInt(25 * 1024 * 1024) + @objc + public static let kMaxFileSizeImage = UInt(6 * 1024 * 1024) + @objc + public static let kMaxFileSizeVideo = UInt(100 * 1024 * 1024) + @objc + public static let kMaxFileSizeAudio = UInt(100 * 1024 * 1024) + @objc + public static let kMaxFileSizeGeneric = UInt(100 * 1024 * 1024) + + @objc + public static let kMaxVideoDimensions: CGFloat = 3 * 1024 + @objc + public static let kMaxAnimatedImageDimensions: UInt = 1 * 1024 + @objc + public static let kMaxStillImageDimensions: UInt = 8 * 1024 } diff --git a/SignalServiceKit/src/Util/NSData+Image.m b/SignalServiceKit/src/Util/NSData+Image.m index d81068645..0a7773de9 100644 --- a/SignalServiceKit/src/Util/NSData+Image.m +++ b/SignalServiceKit/src/Util/NSData+Image.m @@ -2,9 +2,11 @@ // Copyright (c) 2018 Open Whisper Systems. All rights reserved. // -#import "NSData+Image.h" #import "MIMETypeUtil.h" +#import "NSData+Image.h" +#import "OWSFileSystem.h" #import +#import typedef NS_ENUM(NSInteger, ImageFormat) { ImageFormat_Unknown, @@ -24,11 +26,23 @@ typedef NS_ENUM(NSInteger, ImageFormat) { - (BOOL)ows_isValidImage { - if (![self ows_isValidImageWithMimeType:nil]) { + ImageFormat imageFormat = [self ows_guessImageFormat]; + + BOOL isAnimated = imageFormat == ImageFormat_Gif; + + const NSUInteger kMaxFileSize + = (isAnimated ? OWSMediaUtils.kMaxFileSizeAnimatedImage : OWSMediaUtils.kMaxFileSizeImage); + NSUInteger fileSize = self.length; + if (fileSize > kMaxFileSize) { + DDLogWarn(@"Oversize image."); return NO; } - if (![self ows_hasValidImageDimensions]) { + if (![self ows_isValidImageWithMimeType:nil imageFormat:imageFormat]) { + return NO; + } + + if (![self ows_hasValidImageDimensions:isAnimated]) { return NO; } @@ -37,6 +51,37 @@ typedef NS_ENUM(NSInteger, ImageFormat) { + (BOOL)ows_isValidImageAtPath:(NSString *)filePath mimeType:(nullable NSString *)mimeType { + if (mimeType.length < 1) { + NSString *fileExtension = [filePath pathExtension].lowercaseString; + mimeType = [MIMETypeUtil mimeTypeForFileExtension:fileExtension]; + } + if (mimeType.length < 1) { + DDLogError(@"Image has unknown MIME type."); + return NO; + } + NSNumber *_Nullable fileSize = [OWSFileSystem fileSizeOfPath:filePath]; + if (!fileSize) { + DDLogError(@"Could not determine file size."); + return NO; + } + + BOOL isAnimated = NO; + if ([MIMETypeUtil isSupportedAnimatedMIMEType:mimeType]) { + isAnimated = YES; + if (fileSize.unsignedIntegerValue > OWSMediaUtils.kMaxFileSizeAnimatedImage) { + DDLogWarn(@"Oversize animated image."); + return NO; + } + } else if ([MIMETypeUtil isSupportedImageMIMEType:mimeType]) { + if (fileSize.unsignedIntegerValue > OWSMediaUtils.kMaxFileSizeImage) { + DDLogWarn(@"Oversize still image."); + return NO; + } + } else { + DDLogError(@"Image has unsupported MIME type."); + return NO; + } + NSError *error = nil; NSData *_Nullable data = [NSData dataWithContentsOfFile:filePath options:NSDataReadingMappedIfSafe error:&error]; if (!data || error) { @@ -48,7 +93,7 @@ typedef NS_ENUM(NSInteger, ImageFormat) { return NO; } - if (![self ows_hasValidImageDimensionsAtPath:filePath]) { + if (![self ows_hasValidImageDimensionsAtPath:filePath isAnimated:isAnimated]) { DDLogError(@"%@ image had invalid dimensions.", self.logTag); return NO; } @@ -56,18 +101,18 @@ typedef NS_ENUM(NSInteger, ImageFormat) { return YES; } -- (BOOL)ows_hasValidImageDimensions +- (BOOL)ows_hasValidImageDimensions:(BOOL)isAnimated { CGImageSourceRef imageSource = CGImageSourceCreateWithData((__bridge CFDataRef)self, NULL); if (imageSource == NULL) { return NO; } - BOOL result = [NSData ows_hasValidImageDimensionWithImageSource:imageSource]; + BOOL result = [NSData ows_hasValidImageDimensionWithImageSource:imageSource isAnimated:isAnimated]; CFRelease(imageSource); return result; } -+ (BOOL)ows_hasValidImageDimensionsAtPath:(NSString *)path ++ (BOOL)ows_hasValidImageDimensionsAtPath:(NSString *)path isAnimated:(BOOL)isAnimated { NSURL *url = [NSURL fileURLWithPath:path]; if (!url) { @@ -78,12 +123,12 @@ typedef NS_ENUM(NSInteger, ImageFormat) { if (imageSource == NULL) { return NO; } - BOOL result = [self ows_hasValidImageDimensionWithImageSource:imageSource]; + BOOL result = [self ows_hasValidImageDimensionWithImageSource:imageSource isAnimated:isAnimated]; CFRelease(imageSource); return result; } -+ (BOOL)ows_hasValidImageDimensionWithImageSource:(CGImageSourceRef)imageSource ++ (BOOL)ows_hasValidImageDimensionWithImageSource:(CGImageSourceRef)imageSource isAnimated:(BOOL)isAnimated { OWSAssert(imageSource); @@ -116,6 +161,7 @@ typedef NS_ENUM(NSInteger, ImageFormat) { return NO; } NSUInteger depthBits = depthNumber.unsignedIntegerValue; + // This should usually be 1. CGFloat depthBytes = (CGFloat)ceil(depthBits / 8.f); /* The color model of the image such as "RGB", "CMYK", "Gray", or "Lab". @@ -136,7 +182,8 @@ typedef NS_ENUM(NSInteger, ImageFormat) { CGFloat bytesPerPixel = kWorseCastComponentsPerPixel * depthBytes; const CGFloat kExpectedBytePerPixel = 4; - const CGFloat kMaxValidImageDimension = 8 * 1024; + CGFloat kMaxValidImageDimension + = (isAnimated ? OWSMediaUtils.kMaxAnimatedImageDimensions : OWSMediaUtils.kMaxStillImageDimensions); CGFloat kMaxBytes = kMaxValidImageDimension * kMaxValidImageDimension * kExpectedBytePerPixel; CGFloat actualBytes = width * height * bytesPerPixel; if (actualBytes > kMaxBytes) { @@ -148,6 +195,12 @@ typedef NS_ENUM(NSInteger, ImageFormat) { } - (BOOL)ows_isValidImageWithMimeType:(nullable NSString *)mimeType +{ + ImageFormat imageFormat = [self ows_guessImageFormat]; + return [self ows_isValidImageWithMimeType:mimeType imageFormat:imageFormat]; +} + +- (BOOL)ows_isValidImageWithMimeType:(nullable NSString *)mimeType imageFormat:(ImageFormat)imageFormat { // Don't trust the file extension; iOS (e.g. UIKit, Core Graphics) will happily // load a .gif with a .png file extension. @@ -156,7 +209,6 @@ typedef NS_ENUM(NSInteger, ImageFormat) { // // If the image has a declared MIME type, ensure that agrees with the // deduced image format. - ImageFormat imageFormat = [self ows_guessImageFormat]; switch (imageFormat) { case ImageFormat_Unknown: return NO; diff --git a/SignalServiceKit/src/Util/UIImage+OWS.m b/SignalServiceKit/src/Util/UIImage+OWS.m index 02b7cb001..97f9ad367 100644 --- a/SignalServiceKit/src/Util/UIImage+OWS.m +++ b/SignalServiceKit/src/Util/UIImage+OWS.m @@ -63,7 +63,11 @@ } UIGraphicsBeginImageContext(CGSizeMake(thumbnailSize.width, thumbnailSize.height)); - CGContextRef context = UIGraphicsGetCurrentContext(); + CGContextRef _Nullable context = UIGraphicsGetCurrentContext(); + if (context == NULL) { + DDLogError(@"Couldn't create context."); + return nil; + } CGContextSetInterpolationQuality(context, kCGInterpolationHigh); [self drawInRect:CGRectMake(0, 0, thumbnailSize.width, thumbnailSize.height)]; UIImage *_Nullable resized = UIGraphicsGetImageFromCurrentImageContext(); From a2fe4dbe39541b2c235d8dee62d7c3695722f138 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 6 Sep 2018 13:29:14 -0400 Subject: [PATCH 2/3] Refine image validation. --- .../src/Messages/Attachments/OWSThumbnailService.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift index 77e7353a3..165177b0d 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift +++ b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift @@ -172,6 +172,7 @@ private struct OWSThumbnailRequest { } catch let error as NSError { throw OWSThumbnailError.externalError(description: "File write failed: \(thumbnailPath), \(error)", underlyingError: error) } + OWSFileSystem.protectFileOrFolder(atPath: thumbnailPath) return OWSLoadedThumbnail(image: thumbnailImage, data: thumbnailData) } } From 9fefdd2e2c6e59f39b256c5ac2ae5d77f19fff01 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 6 Sep 2018 15:59:19 -0400 Subject: [PATCH 3/3] Respond to CR. --- SignalServiceKit/src/Util/NSData+Image.m | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/SignalServiceKit/src/Util/NSData+Image.m b/SignalServiceKit/src/Util/NSData+Image.m index 0a7773de9..ab1e0a568 100644 --- a/SignalServiceKit/src/Util/NSData+Image.m +++ b/SignalServiceKit/src/Util/NSData+Image.m @@ -42,7 +42,7 @@ typedef NS_ENUM(NSInteger, ImageFormat) { return NO; } - if (![self ows_hasValidImageDimensions:isAnimated]) { + if (![self ows_hasValidImageDimensionsWithIsAnimated:isAnimated]) { return NO; } @@ -65,9 +65,8 @@ typedef NS_ENUM(NSInteger, ImageFormat) { return NO; } - BOOL isAnimated = NO; - if ([MIMETypeUtil isSupportedAnimatedMIMEType:mimeType]) { - isAnimated = YES; + BOOL isAnimated = [MIMETypeUtil isSupportedAnimatedMIMEType:mimeType]; + if (isAnimated) { if (fileSize.unsignedIntegerValue > OWSMediaUtils.kMaxFileSizeAnimatedImage) { DDLogWarn(@"Oversize animated image."); return NO; @@ -101,7 +100,7 @@ typedef NS_ENUM(NSInteger, ImageFormat) { return YES; } -- (BOOL)ows_hasValidImageDimensions:(BOOL)isAnimated +- (BOOL)ows_hasValidImageDimensionsWithIsAnimated:(BOOL)isAnimated { CGImageSourceRef imageSource = CGImageSourceCreateWithData((__bridge CFDataRef)self, NULL); if (imageSource == NULL) {