From 034865481a970ae02d280d615b26d0e926465469 Mon Sep 17 00:00:00 2001 From: Nick Bourdakos Date: Tue, 22 Aug 2023 11:35:54 -0700 Subject: [PATCH] Sanitize incoming file names PiperOrigin-RevId: 559175545 --- .../platform/implementation/apple/Tests/BUILD | 1 + .../apple/Tests/GNCPlatformTest.mm | 83 +++++++++++++++++++ .../platform/implementation/apple/platform.mm | 40 +++++---- 3 files changed, 108 insertions(+), 16 deletions(-) create mode 100644 internal/platform/implementation/apple/Tests/GNCPlatformTest.mm diff --git a/internal/platform/implementation/apple/Tests/BUILD b/internal/platform/implementation/apple/Tests/BUILD index 2ca87d00..9cc10150 100644 --- a/internal/platform/implementation/apple/Tests/BUILD +++ b/internal/platform/implementation/apple/Tests/BUILD @@ -42,6 +42,7 @@ objc_library( "GNCFakePeripheralManager.m", "GNCIPAddressTest.mm", "GNCMultiThreadExecutorTest.mm", + "GNCPlatformTest.mm", "GNCScheduledExecutorTest.mm", "GNCSingleThreadExecutorTest.mm", "GNCUtilsTest.mm", diff --git a/internal/platform/implementation/apple/Tests/GNCPlatformTest.mm b/internal/platform/implementation/apple/Tests/GNCPlatformTest.mm new file mode 100644 index 00000000..322d0725 --- /dev/null +++ b/internal/platform/implementation/apple/Tests/GNCPlatformTest.mm @@ -0,0 +1,83 @@ +// Copyright 2023 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "internal/platform/implementation/platform.h" + +#import +#import + +void GNCEnsureFileAtPath(std::string path) { + [NSFileManager.defaultManager + createDirectoryAtPath:[@(path.c_str()) stringByDeletingLastPathComponent] + withIntermediateDirectories:YES + attributes:nil + error:nil]; + [[NSData data] writeToFile:@(path.c_str()) options:0 error:nil]; +} + +@interface GNCPlatformTest : XCTestCase +@end + +@implementation GNCPlatformTest + +- (void)testGetCustomSavePath { + NSString *expected = [NSURL fileURLWithPath:@"a/b/c.d"].path; + std::string actual = nearby::api::ImplementationPlatform::GetCustomSavePath("a/b", "c.d"); + XCTAssertEqualObjects(@(actual.c_str()), expected); +} + +- (void)testGetCustomSavePathWithIllegalCharacters { + NSString *expected = [NSURL fileURLWithPath:@"a/b/Fi?le*: Name.ext"].path; + std::string actual = + nearby::api::ImplementationPlatform::GetCustomSavePath("a/b", "Fi?le*/ Name.ext"); + XCTAssertEqualObjects(@(actual.c_str()), expected); +} + +- (void)testGetCustomSavePathWithPathEscapingCharacters { + NSString *expected = [NSURL fileURLWithPath:@"a/b/..:c:..:d.e"].path; + std::string actual = + nearby::api::ImplementationPlatform::GetCustomSavePath("a/../../b", "../c/../d.e"); + XCTAssertEqualObjects(@(actual.c_str()), expected); +} + +- (void)testGetCustomSavePathDuplicateNames { + NSString *expected1 = [NSURL fileURLWithPath:@"a/b/cat.jpg"].path; + NSString *expected2 = [NSURL fileURLWithPath:@"a/b/cat 2.jpg"].path; + NSString *expected3 = [NSURL fileURLWithPath:@"a/b/cat 3.jpg"].path; + + std::string actual1 = nearby::api::ImplementationPlatform::GetCustomSavePath("a/b", "cat.jpg"); + GNCEnsureFileAtPath(actual1); + + std::string actual2 = nearby::api::ImplementationPlatform::GetCustomSavePath("a/b", "cat.jpg"); + GNCEnsureFileAtPath(actual2); + + std::string actual3 = nearby::api::ImplementationPlatform::GetCustomSavePath("a/b", "cat.jpg"); + + // Cleanup created files. + [NSFileManager.defaultManager removeItemAtPath:@(actual1.c_str()) error:nil]; + [NSFileManager.defaultManager removeItemAtPath:@(actual2.c_str()) error:nil]; + + XCTAssertEqualObjects(@(actual1.c_str()), expected1); + XCTAssertEqualObjects(@(actual2.c_str()), expected2); + XCTAssertEqualObjects(@(actual3.c_str()), expected3); +} + +- (void)testGetDownloadPath { + NSString *expected = + [[NSURL fileURLWithPath:NSTemporaryDirectory()] URLByAppendingPathComponent:@"a/b/c.d"].path; + std::string actual = nearby::api::ImplementationPlatform::GetDownloadPath("a/b", "c.d"); + XCTAssertEqualObjects(@(actual.c_str()), expected); +} + +@end diff --git a/internal/platform/implementation/apple/platform.mm b/internal/platform/implementation/apple/platform.mm index f23a112b..b70fc53a 100644 --- a/internal/platform/implementation/apple/platform.mm +++ b/internal/platform/implementation/apple/platform.mm @@ -44,37 +44,45 @@ namespace api { std::string ImplementationPlatform::GetCustomSavePath(const std::string& parent_folder, const std::string& file_name) { - NSFileManager* manager = [NSFileManager defaultManager]; + // Collapse any path escaping characters. + NSString* parentFolder = [@(parent_folder.c_str()) stringByReplacingOccurrencesOfString:@"../" + withString:@""]; + NSURL* parentFolderURL = [NSURL fileURLWithPath:parentFolder]; - NSURL* parentFolder = [NSURL fileURLWithPath:@(parent_folder.c_str())]; - - NSString* fileName = @(file_name.c_str()); + // The only reserved character in a file name on macOS is the forward-slash. It's unclear if iOS + // has any additional restrictions. + // + // """ + // In the Finder, filenames containing `/` can be created, but `/` is stored as a colon (:) in the + // filesystem, and is shown as such on the command line. Filenames containing `:` created from the + // command line are shown with `/` instead of `:` in the Finder, so that it is impossible to + // create a file that the Finder shows as having a `:` in its filename. + // """ + // + // See: https://en.wikipedia.org/wiki/Filename + NSString* fileName = [@(file_name.c_str()) stringByReplacingOccurrencesOfString:@"/" + withString:@":"]; NSString* baseName = [fileName stringByDeletingPathExtension]; NSString* extension = [fileName pathExtension]; - NSURL* url = [parentFolder URLByAppendingPathComponent:fileName]; + NSURL* url = [parentFolderURL URLByAppendingPathComponent:fileName]; NSInteger index = 1; - while ([manager fileExistsAtPath:url.path]) { + while ([NSFileManager.defaultManager fileExistsAtPath:url.path]) { index++; NSString* fileName = [NSString stringWithFormat:@"%@ %@.%@", baseName, [@(index) stringValue], extension]; - url = [parentFolder URLByAppendingPathComponent:fileName]; + url = [parentFolderURL URLByAppendingPathComponent:fileName]; } - return [url.path UTF8String]; + return url.path.UTF8String; } std::string ImplementationPlatform::GetDownloadPath(const std::string& parent_folder, const std::string& file_name) { - // TODO(jfcarroll): This needs to be done correctly, we now have a file name and parent folder, - // they should be combined with the default download path - NSString* fileName = ObjCStringFromCppString(file_name); - - // TODO(b/227535777): If file name matches an existing file, it will be overwritten. Append a - // number until a unique file name is reached 'foobar (2).png'. - - return CppStringFromObjCString([NSTemporaryDirectory() stringByAppendingPathComponent:fileName]); + NSString* customSavePath = + [NSTemporaryDirectory() stringByAppendingPathComponent:@(parent_folder.c_str())]; + return GetCustomSavePath(customSavePath.UTF8String, file_name); } OSName ImplementationPlatform::GetCurrentOS() { return OSName::kApple; }