Attachment 370158 Details for Bug 197898 – Patch

[patch] Patch

bug-197898-20190518001046.patch (text/plain), 16.35 KB, created by Tadeu Zagallo on 2019-05-17 15:10:48 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-05-17 15:10:48 PDT

Size: 16.35 KB

patch

obsolete

>Subversion Revision: 245304
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index e732524d691b3eedbc0b97a881cd68b069535e73..b1cbbb5b105df5342fb1714363f65d378e100ffe 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-05-15  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        Only cache bytecode for API clients in data vaults
>+        https://bugs.webkit.org/show_bug.cgi?id=197898
>+        <rdar://problem/45945449>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        For security reasons, enforce that API clients only store cached bytecode in data vaults.
>+
>+        * API/JSScript.mm:
>+        (validateBytecodeCachePath):
>+        (+[JSScript scriptOfType:withSource:andSourceURL:andBytecodeCache:inVirtualMachine:error:]):
>+        (+[JSScript scriptOfType:memoryMappedFromASCIIFile:withSourceURL:andBytecodeCache:inVirtualMachine:error:]):
>+        * API/tests/testapi.mm:
>+        (testModuleBytecodeCache):
>+        (testProgramBytecodeCache):
>+        (testBytecodeCacheWithSyntaxError):
>+        (testBytecodeCacheWithSameCacheFileAndDifferentScript):
>+        (testCacheFileFailsWhenItsAlreadyCached):
>+        (testCanCacheManyFilesWithTheSameVM):
>+        (testIsUsingBytecodeCacheAccessor):
>+        (testBytecodeCacheValidation):
>+        (testObjectiveCAPI):
>+        * runtime/Options.h:
>+
> 2019-05-14  Ross Kirsling  <ross.kirsling@sony.com>
> 
>         Unreviewed restoration of non-unified build.
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 6bc60aa7e4bf74aa275461c4ac185caa4d982601..0ebd8cdb141449789c82bb6dc97411161495f051 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-05-15  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        Only cache bytecode for API clients in data vaults
>+        https://bugs.webkit.org/show_bug.cgi?id=197898
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add SPI to check if a filesystem path is restricted as a data vault.
>+
>+        * WTF.xcodeproj/project.pbxproj:
>+        * wtf/spi/darwin/DataVaultSPI.h: Added.
>+
> 2019-05-14  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r245281.
>diff --git a/Source/JavaScriptCore/API/JSScript.mm b/Source/JavaScriptCore/API/JSScript.mm
>index bdbd682283f36c6514cf2736ffdaa012c85120cf..6cf44bb70bb12552ca0eac319e5d237dd02f1285 100644
>--- a/Source/JavaScriptCore/API/JSScript.mm
>+++ b/Source/JavaScriptCore/API/JSScript.mm
>@@ -38,8 +38,10 @@
> #import "ParserError.h"
> #import "Symbol.h"
> #include <sys/stat.h>
>+#include <wtf/FileMetadata.h>
> #include <wtf/FileSystem.h>
> #include <wtf/Scope.h>
>+#include <wtf/spi/darwin/DataVaultSPI.h>
> 
> #if JSC_OBJC_API_ENABLED
> 
>@@ -60,9 +62,50 @@ static JSScript *createError(NSString *message, NSError** error)
>     return nil;
> }
> 
>+static bool validateBytecodeCachePath(NSURL* cachePath, NSError** error)
>+{
>+    if (!cachePath)
>+        return true;
>+
>+    URL cachePathURL([cachePath absoluteURL]);
>+    if (!cachePathURL.isLocalFile()) {
>+        createError([NSString stringWithFormat:@"Cache path `%@` is not a local file", static_cast<NSString *>(cachePathURL)], error);
>+        return false;
>+    }
>+
>+    String systemPath = cachePathURL.fileSystemPath();
>+
>+    if (auto metadata = FileSystem::fileMetadata(systemPath)) {
>+        if (metadata->type != FileMetadata::Type::File) {
>+            createError([NSString stringWithFormat:@"Cache path `%s` already exists and is not a file", systemPath.utf8().data()], error);
>+            return false;
>+        }
>+    }
>+
>+    String directory = FileSystem::directoryName(systemPath);
>+    if (directory.isNull()) {
>+        createError([NSString stringWithFormat:@"Cache path `%s` does not contain in a valid directory", systemPath.utf8().data()], error);
>+        return false;
>+    }
>+
>+    if (!FileSystem::fileIsDirectory(directory, FileSystem::ShouldFollowSymbolicLinks::No)) {
>+        createError([NSString stringWithFormat:@"Cache directory `%s` is not a directory or does not exist", directory.utf8().data()], error);
>+        return false;
>+    }
>+
>+    if (LIKELY(!JSC::Options::disableBytecodeCacheDataVaultEnforcement()) && rootless_check_datavault_flag(FileSystem::fileSystemRepresentation(directory).data(), nullptr)) {
>+        createError([NSString stringWithFormat:@"Cache directory `%s` is not a data vault", directory.utf8().data()], error);
>+        return false;
>+    }
>+
>+    return true;
>+}
>+
> + (instancetype)scriptOfType:(JSScriptType)type withSource:(NSString *)source andSourceURL:(NSURL *)sourceURL andBytecodeCache:(NSURL *)cachePath inVirtualMachine:(JSVirtualMachine *)vm error:(out NSError **)error
> {
>-    UNUSED_PARAM(error);
>+    if (!validateBytecodeCachePath(cachePath, error))
>+        return nil;
>+
>     JSScript *result = [[[JSScript alloc] init] autorelease];
>     result->m_virtualMachine = vm;
>     result->m_type = type;
>@@ -75,7 +118,9 @@ + (instancetype)scriptOfType:(JSScriptType)type withSource:(NSString *)source an
> 
> + (instancetype)scriptOfType:(JSScriptType)type memoryMappedFromASCIIFile:(NSURL *)filePath withSourceURL:(NSURL *)sourceURL andBytecodeCache:(NSURL *)cachePath inVirtualMachine:(JSVirtualMachine *)vm error:(out NSError **)error
> {
>-    UNUSED_PARAM(error);
>+    if (!validateBytecodeCachePath(cachePath, error))
>+        return nil;
>+
>     URL filePathURL([filePath absoluteURL]);
>     if (!filePathURL.isLocalFile())
>         return createError([NSString stringWithFormat:@"File path %@ is not a local file", static_cast<NSString *>(filePathURL)], error);
>diff --git a/Source/JavaScriptCore/API/tests/testapi.mm b/Source/JavaScriptCore/API/tests/testapi.mm
>index 5649677bdb017e254c09676d63b6a1711330c0a1..1a86809b052728576d1729535552411f618008d8 100644
>--- a/Source/JavaScriptCore/API/tests/testapi.mm
>+++ b/Source/JavaScriptCore/API/tests/testapi.mm
>@@ -2078,6 +2078,7 @@ static NSURL *tempFile(NSString *string)
> 
> static void testModuleBytecodeCache()
> {
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = true;
>     @autoreleasepool {
>         NSString *fooSource = @"import 'otherDirectory/baz.js'; export let n = null;";
>         NSString *barSource = @"import { n } from '../foo.js'; export let foo = () => n;";
>@@ -2136,10 +2137,12 @@ static void testModuleBytecodeCache()
>         removedAll &= [fileManager removeItemAtURL:bazCachePath error:nil];
>         checkResult(@"Removed all temp files created", removedAll);
>     }
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = false;
> }
> 
> static void testProgramBytecodeCache()
> {
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = true;
>     @autoreleasepool {
>         NSString *fooSource = @"function foo() { return 42; }; function bar() { return 40; }; foo() + bar();";
>         NSURL *fooCachePath = tempFile(@"foo.js.cache");
>@@ -2160,10 +2163,12 @@ static void testProgramBytecodeCache()
>         BOOL removedAll = [fileManager removeItemAtURL:fooCachePath error:nil];
>         checkResult(@"Removed all temp files created", removedAll);
>     }
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = false;
> }
> 
> static void testBytecodeCacheWithSyntaxError(JSScriptType type)
> {
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = true;
>     @autoreleasepool {
>         NSString *fooSource = @"this is a syntax error";
>         NSURL *fooCachePath = tempFile(@"foo.js.cache");
>@@ -2176,10 +2181,13 @@ static void testBytecodeCacheWithSyntaxError(JSScriptType type)
>         RELEASE_ASSERT(error);
>         checkResult(@"Got error when trying to cache bytecode for a script with a syntax error.", [[error description] containsString:@"Unable to generate bytecode for this JSScript because of a parser error"]);
>     }
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = false;
> }
> 
> static void testBytecodeCacheWithSameCacheFileAndDifferentScript(bool forceDiskCache)
> {
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = true;
>+
>     NSURL *cachePath = tempFile(@"cachePath.cache");
>     NSURL *sourceURL = [NSURL URLWithString:@"my-path"];
> 
>@@ -2220,6 +2228,8 @@ static void testBytecodeCacheWithSameCacheFileAndDifferentScript(bool forceDiskC
>     NSFileManager* fileManager = [NSFileManager defaultManager];
>     BOOL removedAll = [fileManager removeItemAtURL:cachePath error:nil];
>     checkResult(@"Removed all temp files created", removedAll);
>+
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = false;
> }
> 
> static void testProgramJSScriptException()
>@@ -2245,6 +2255,7 @@ static void testProgramJSScriptException()
> 
> static void testCacheFileFailsWhenItsAlreadyCached()
> {
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = true;
>     NSURL* cachePath = tempFile(@"foo.program.cache");
>     NSURL* sourceURL = [NSURL URLWithString:@"my-path"];
>     NSString *source = @"function foo() { return 42; } foo();";
>@@ -2277,10 +2288,12 @@ static void testCacheFileFailsWhenItsAlreadyCached()
>     NSFileManager* fileManager = [NSFileManager defaultManager];
>     BOOL removedAll = [fileManager removeItemAtURL:cachePath error:nil];
>     checkResult(@"Successfully removed cache file", removedAll);
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = false;
> }
> 
> static void testCanCacheManyFilesWithTheSameVM()
> {
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = true;
>     NSMutableArray *cachePaths = [[NSMutableArray alloc] init];
>     NSMutableArray *scripts = [[NSMutableArray alloc] init];
> 
>@@ -2319,10 +2332,12 @@ static void testCanCacheManyFilesWithTheSameVM()
>         removedAll &= [fileManager removeItemAtURL:path error:nil];
> 
>     checkResult(@"Removed all cache files", removedAll);
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = false;
> }
> 
> static void testIsUsingBytecodeCacheAccessor()
> {
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = true;
>     NSURL* cachePath = tempFile(@"foo.program.cache");
>     NSURL* sourceURL = [NSURL URLWithString:@"my-path"];
>     NSString *source = @"function foo() { return 1337; } foo();";
>@@ -2354,6 +2369,29 @@ static void testIsUsingBytecodeCacheAccessor()
>     NSFileManager* fileManager = [NSFileManager defaultManager];
>     BOOL removedAll = [fileManager removeItemAtURL:cachePath error:nil];
>     checkResult(@"Successfully removed cache file", removedAll);
>+    JSC::Options::disableBytecodeCacheDataVaultEnforcement() = false;
>+}
>+
>+static void testBytecodeCacheValidation()
>+{
>+    @autoreleasepool {
>+        JSContext *context = [[JSContext alloc] init];
>+
>+        auto testURL = [&](NSURL* cacheURL, NSString* expectedErrorMessage)
>+        {
>+            NSError* error;
>+            JSScript *script = [JSScript scriptOfType:kJSScriptTypeProgram withSource:@"" andSourceURL:[NSURL URLWithString:@"my-path"] andBytecodeCache:cacheURL inVirtualMachine:context.virtualMachine error:&error];
>+            RELEASE_ASSERT(!script);
>+            RELEASE_ASSERT(error);
>+            NSString* testDesciption = [NSString stringWithFormat:@"Cache path validation for `%@` fails with message `%@`", cacheURL.absoluteString, expectedErrorMessage];
>+            checkResult(testDesciption, [error.description containsString:expectedErrorMessage]);
>+        };
>+
>+        testURL([NSURL URLWithString:@""], @"Cache path `` is not a local file");
>+        testURL([NSURL URLWithString:@"file:///"], @"Cache path `/` already exists and is not a file");
>+        testURL([NSURL URLWithString:@"file:///a/b/c/d/e"], @"Cache directory `/a/b/c/d` is not a directory or does not exist");
>+        testURL([NSURL URLWithString:@"file:///private/tmp/file.cache"], @"Cache directory `/private/tmp` is not a data vault");
>+    }
> }
> 
> @interface JSContextFileLoaderDelegate : JSContext <JSModuleLoaderDelegate>
>@@ -2628,6 +2666,7 @@ void testObjectiveCAPI(const char* filter)
>     RUN(testCacheFileFailsWhenItsAlreadyCached());
>     RUN(testCanCacheManyFilesWithTheSameVM());
>     RUN(testIsUsingBytecodeCacheAccessor());
>+    RUN(testBytecodeCacheValidation());
> 
>     RUN(testLoaderRejectsNilScriptURL());
>     RUN(testLoaderRejectsFailedFetch());
>diff --git a/Source/JavaScriptCore/runtime/Options.h b/Source/JavaScriptCore/runtime/Options.h
>index 6bc150021a3a5003e8bb14a482ce1930562fc5d8..8de026f513636294e7ef83300bf97a388f2e9430 100644
>--- a/Source/JavaScriptCore/runtime/Options.h
>+++ b/Source/JavaScriptCore/runtime/Options.h
>@@ -518,6 +518,7 @@ constexpr bool enableWebAssemblyStreamingApi = false;
>     v(bool, validateAbstractInterpreterState, false, Restricted, nullptr) \
>     v(double, validateAbstractInterpreterStateProbability, 0.5, Normal, nullptr) \
>     v(optionString, dumpJITMemoryPath, nullptr, Restricted, nullptr) \
>+    v(bool, disableBytecodeCacheDataVaultEnforcement, false, Restricted, "Disablement enforcement that the bytecode cache must be stored in a data vault for testing purposes") \
> 
> 
> enum OptionEquivalence {
>diff --git a/Source/WTF/WTF.xcodeproj/project.pbxproj b/Source/WTF/WTF.xcodeproj/project.pbxproj
>index 30204fcd8882fa155c61745a3f33d892689b787b..8824535a3622581a8d87144b6b10700d47117203 100644
>--- a/Source/WTF/WTF.xcodeproj/project.pbxproj
>+++ b/Source/WTF/WTF.xcodeproj/project.pbxproj
>@@ -288,6 +288,7 @@
> 		1469419416EAAFF80024E146 /* SchedulePair.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SchedulePair.h; sourceTree = "<group>"; };
> 		1469419A16EAB10A0024E146 /* AutodrainedPool.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AutodrainedPool.h; sourceTree = "<group>"; };
> 		1469419B16EAB10A0024E146 /* AutodrainedPool.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = AutodrainedPool.cpp; sourceTree = "<group>"; };
>+		14933E21228C22DF00F79E46 /* DataVaultSPI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DataVaultSPI.h; sourceTree = "<group>"; };
> 		149EF16216BBFE0D000A4331 /* TriState.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TriState.h; sourceTree = "<group>"; };
> 		14E785E71DFB330100209BD1 /* OrdinalNumber.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OrdinalNumber.h; sourceTree = "<group>"; };
> 		14F3B0F615E45E4600210069 /* SaturatedArithmetic.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SaturatedArithmetic.h; sourceTree = "<group>"; };
>@@ -1363,6 +1364,7 @@
> 		CE73E02319DCB7AB00580D5C /* darwin */ = {
> 			isa = PBXGroup;
> 			children = (
>+				14933E21228C22DF00F79E46 /* DataVaultSPI.h */,
> 				E431CC4A21187ADB000C8A07 /* DispatchSPI.h */,
> 				93DDE9311CDC052D00FD3491 /* dyldSPI.h */,
> 				A5098AFF1C169E0700087797 /* SandboxSPI.h */,
>diff --git a/Source/WTF/wtf/spi/darwin/DataVaultSPI.h b/Source/WTF/wtf/spi/darwin/DataVaultSPI.h
>new file mode 100644
>index 0000000000000000000000000000000000000000..944bef926c90c12f4f7d16cd9469f64f29ca9576
>--- /dev/null
>+++ b/Source/WTF/wtf/spi/darwin/DataVaultSPI.h
>@@ -0,0 +1,40 @@
>+/*
>+ * Copyright (C) 2019 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#pragma once
>+
>+#if OS(DARWIN)
>+
>+#if USE(APPLE_INTERNAL_SDK)
>+#import <rootless.h>
>+#endif
>+
>+WTF_EXTERN_C_BEGIN
>+
>+int rootless_check_datavault_flag(const char *path, const char *storage_class);
>+
>+WTF_EXTERN_C_END
>+
>+#endif // OS(DARWIN)

Actions: View | Formatted Diff | Diff

Attachments on bug 197898: 369910 | 369958 | 370158 | 370220 | 370300 | 370333