Title: [260081] trunk
Revision
260081
Author
bfulg...@apple.com
Date
2020-04-14 11:07:24 -0700 (Tue, 14 Apr 2020)

Log Message

InjectedBundle parameters often need initialization function called before unarchiving
https://bugs.webkit.org/show_bug.cgi?id=189709
<rdar://problem/44573653>

Reviewed by Ryosuke Niwa.

Source/WebKit:

Handle the case where the InjectedBundle parameters do not successfully decode because they contain
an unexpected class from the embedding program. If this happens, try decoding the bundle parameters
after the bundle initialiation function runs, which gives the embedding program the opportunity to
register additional classes that are safe for serialization.

Extend WKWebProcessPlugIn with a method that returns the names of any custom classes that need
to be serialized by the InjectedBundle.

Create a new 'decodeBundleParameters' method that contains the logic that used to live in 'initialize'.
Revise 'initialize' to call this new method.

* WebProcess/InjectedBundle/InjectedBundle.h:
* WebProcess/InjectedBundle/mac/InjectedBundleMac.mm:
(WebKit::InjectedBundle::initialize): Use the new method.
(WebKit::InjectedBundle::decodeBundleParameters): Added.
(WebKit::InjectedBundle::setBundleParameters): Use 'decodeObjectOfClasses' with the more complete
'classesForCoder' method to unarchive the passed bundle parameters, rather than the
NSDictionary-specific method, since InjectedBundles often encode other types of objects, and the
NSDictionary object may itself hold other kinds of objects.
* WebProcess/InjectedBundle/API/mac/WKWebProcessPlugIn.h:
(WebKit::WKWebProcessPlugIn::additionalClassesForParameterCoder): Added.

Tools:

* TestWebKitAPI/cocoa/WebProcessPlugIn/WebProcessPlugIn.mm:
(-[WebProcessPlugIn additionalClassesForParameterCoder]): Added.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (260080 => 260081)


--- trunk/Source/WebKit/ChangeLog	2020-04-14 17:17:45 UTC (rev 260080)
+++ trunk/Source/WebKit/ChangeLog	2020-04-14 18:07:24 UTC (rev 260081)
@@ -1,3 +1,33 @@
+2020-04-14  Brent Fulgham  <bfulg...@apple.com>
+
+        InjectedBundle parameters often need initialization function called before unarchiving
+        https://bugs.webkit.org/show_bug.cgi?id=189709
+        <rdar://problem/44573653>
+
+        Reviewed by Ryosuke Niwa.
+
+        Handle the case where the InjectedBundle parameters do not successfully decode because they contain
+        an unexpected class from the embedding program. If this happens, try decoding the bundle parameters
+        after the bundle initialiation function runs, which gives the embedding program the opportunity to
+        register additional classes that are safe for serialization.
+
+        Extend WKWebProcessPlugIn with a method that returns the names of any custom classes that need
+        to be serialized by the InjectedBundle.
+        
+        Create a new 'decodeBundleParameters' method that contains the logic that used to live in 'initialize'.
+        Revise 'initialize' to call this new method.
+
+        * WebProcess/InjectedBundle/InjectedBundle.h:
+        * WebProcess/InjectedBundle/mac/InjectedBundleMac.mm:
+        (WebKit::InjectedBundle::initialize): Use the new method.
+        (WebKit::InjectedBundle::decodeBundleParameters): Added.
+        (WebKit::InjectedBundle::setBundleParameters): Use 'decodeObjectOfClasses' with the more complete
+        'classesForCoder' method to unarchive the passed bundle parameters, rather than the
+        NSDictionary-specific method, since InjectedBundles often encode other types of objects, and the
+        NSDictionary object may itself hold other kinds of objects.
+        * WebProcess/InjectedBundle/API/mac/WKWebProcessPlugIn.h:
+        (WebKit::WKWebProcessPlugIn::additionalClassesForParameterCoder): Added.
+
 2020-04-14  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Address review comments after r260035

Modified: trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundleInitialize.h (260080 => 260081)


--- trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundleInitialize.h	2020-04-14 17:17:45 UTC (rev 260080)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundleInitialize.h	2020-04-14 18:07:24 UTC (rev 260081)
@@ -23,8 +23,7 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef WKBundleInitialize_h
-#define WKBundleInitialize_h
+#pragma once
 
 #include <WebKit/WKBase.h>
 
@@ -35,8 +34,9 @@
 // NOTE: Must be implemented by InjectedBundle's as a function named "WKBundleInitialize".
 typedef void (*WKBundleInitializeFunctionPtr)(WKBundleRef, WKTypeRef);
 
+// NOTE: Must be implemented by InjectedBundle's as a function named "WKBundleAdditionalClassesForParameterCoder".
+typedef void (*WKBundleAdditionalClassesForParameterCoderFunctionPtr)(WKBundleRef, WKTypeRef);
+
 #ifdef __cplusplus
 }
 #endif
-
-#endif /* WKBundleInitialize_h */

Modified: trunk/Source/WebKit/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugIn.h (260080 => 260081)


--- trunk/Source/WebKit/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugIn.h	2020-04-14 17:17:45 UTC (rev 260080)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugIn.h	2020-04-14 18:07:24 UTC (rev 260081)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2020 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -37,6 +37,7 @@
 - (void)webProcessPlugIn:(WKWebProcessPlugInController *)plugInController initializeWithObject:(id)initializationObject;
 - (void)webProcessPlugIn:(WKWebProcessPlugInController *)plugInController didCreateBrowserContextController:(WKWebProcessPlugInBrowserContextController *)browserContextController;
 - (void)webProcessPlugIn:(WKWebProcessPlugInController *)plugInController willDestroyBrowserContextController:(WKWebProcessPlugInBrowserContextController *)browserContextController;
+- (NSArray *)additionalClassesForParameterCoder;
 @end
 
 WK_CLASS_AVAILABLE(macos(10.10), ios(8.0))

Modified: trunk/Source/WebKit/WebProcess/InjectedBundle/InjectedBundle.h (260080 => 260081)


--- trunk/Source/WebKit/WebProcess/InjectedBundle/InjectedBundle.h	2020-04-14 17:17:45 UTC (rev 260080)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/InjectedBundle.h	2020-04-14 18:07:24 UTC (rev 260081)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2020 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -156,6 +156,10 @@
 private:
     explicit InjectedBundle(const WebProcessCreationParameters&);
 
+#if PLATFORM(COCOA)
+    bool decodeBundleParameters(API::Data*);
+#endif
+
     String m_path;
     PlatformBundle m_platformBundle; // This is leaked right now, since we never unload the bundle/module.
 

Modified: trunk/Source/WebKit/WebProcess/InjectedBundle/mac/InjectedBundleMac.mm (260080 => 260081)


--- trunk/Source/WebKit/WebProcess/InjectedBundle/mac/InjectedBundleMac.mm	2020-04-14 17:17:45 UTC (rev 260080)
+++ trunk/Source/WebKit/WebProcess/InjectedBundle/mac/InjectedBundleMac.mm	2020-04-14 18:07:24 UTC (rev 260081)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2020 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -92,6 +92,27 @@
     return createUnarchiver(data.data(), data.size());
 }
 
+bool InjectedBundle::decodeBundleParameters(API::Data* bundleParameterDataPtr)
+{
+    if (!bundleParameterDataPtr)
+        return true;
+
+    auto unarchiver = createUnarchiver(*bundleParameterDataPtr);
+
+    NSDictionary *dictionary = nil;
+    @try {
+        dictionary = [unarchiver.get() decodeObjectOfClasses:classesForCoder() forKey:@"parameters"];
+        ASSERT([dictionary isKindOfClass:[NSDictionary class]]);
+    } @catch (NSException *exception) {
+        LOG_ERROR("Failed to decode bundle parameters: %@." , exception);
+        return false;
+    }
+    
+    ASSERT(!m_bundleParameters || m_bundleParameters.get());
+    m_bundleParameters = adoptNS([[WKWebProcessBundleParameters alloc] initWithDictionary:dictionary]);
+    return true;
+}
+
 bool InjectedBundle::initialize(const WebProcessCreationParameters& parameters, API::Object* initializationUserData)
 {
     if (m_sandboxExtension) {
@@ -121,6 +142,7 @@
         return false;
     }
 
+    WKBundleAdditionalClassesForParameterCoderFunctionPtr additionalClassesForParameterCoderFunction = nullptr;
     WKBundleInitializeFunctionPtr initializeFunction = nullptr;
     if (RetainPtr<CFURLRef> executableURL = adoptCF(CFBundleCopyExecutableURL([m_platformBundle _cfBundle]))) {
         static constexpr size_t maxPathSize = 4096;
@@ -127,8 +149,10 @@
         char pathToExecutable[maxPathSize];
         if (CFURLGetFileSystemRepresentation(executableURL.get(), true, bitwise_cast<uint8_t*>(pathToExecutable), maxPathSize)) {
             // We don't hold onto this handle anywhere more permanent since we never dlcose.
-            if (void* handle = dlopen(pathToExecutable, RTLD_LAZY | RTLD_GLOBAL | RTLD_FIRST))
+            if (void* handle = dlopen(pathToExecutable, RTLD_LAZY | RTLD_GLOBAL | RTLD_FIRST)) {
+                additionalClassesForParameterCoderFunction = bitwise_cast<WKBundleAdditionalClassesForParameterCoderFunctionPtr>(dlsym(handle, "WKBundleAdditionalClassesForParameterCoder"));
                 initializeFunction = bitwise_cast<WKBundleInitializeFunctionPtr>(dlsym(handle, "WKBundleInitialize"));
+            }
         }
     }
         
@@ -139,19 +163,14 @@
         }
     }
 
-    if (parameters.bundleParameterData) {
-        NSDictionary *dictionary = nil;
-        auto unarchiver = createUnarchiver(*parameters.bundleParameterData);
-        @try {
-            dictionary = [unarchiver decodeObjectOfClass:[NSObject class] forKey:@"parameters"];
-            ASSERT([dictionary isKindOfClass:[NSDictionary class]]);
-        } @catch (NSException *exception) {
-            LOG_ERROR("Failed to decode bundle parameters: %@", exception);
-        }
+    if (!additionalClassesForParameterCoderFunction)
+        additionalClassesForParameterCoderFunction = bitwise_cast<WKBundleAdditionalClassesForParameterCoderFunctionPtr>(CFBundleGetFunctionPointerForName([m_platformBundle _cfBundle], CFSTR("WKBundleAdditionalClassesForParameterCoder")));
 
-        ASSERT(!m_bundleParameters);
-        m_bundleParameters = adoptNS([[WKWebProcessBundleParameters alloc] initWithDictionary:dictionary]);
-    }
+    // Update list of valid classes for the parameter coder
+    if (additionalClassesForParameterCoderFunction)
+        additionalClassesForParameterCoderFunction(toAPI(this), toAPI(initializationUserData));
+
+    decodeBundleParameters(parameters.bundleParameterData.get());
     
 #if ENABLE(WEBPROCESS_WINDOWSERVER_BLOCKING)
     // Swizzle [NSEvent modiferFlags], since it always returns 0 when the WindowServer is blocked.
@@ -189,6 +208,11 @@
     WKWebProcessPlugInController* plugInController = WebKit::wrapper(*this);
     [plugInController _setPrincipalClassInstance:instance];
 
+    if ([instance respondsToSelector:@selector(additionalClassesForParameterCoder)]) {
+        [plugInController extendClassesForParameterCoder:[instance additionalClassesForParameterCoder]];
+        decodeBundleParameters(parameters.bundleParameterData.get());
+    }
+
     if ([instance respondsToSelector:@selector(webProcessPlugIn:initializeWithObject:)]) {
         RetainPtr<id> objCInitializationUserData;
         if (initializationUserData && initializationUserData->type() == API::Object::Type::ObjCObjectGraph)
@@ -265,7 +289,7 @@
     NSDictionary *parameters = nil;
     auto unarchiver = createUnarchiver(value);
     @try {
-        parameters = [unarchiver decodeObjectOfClass:[NSDictionary class] forKey:@"parameters"];
+        parameters = [unarchiver decodeObjectOfClasses:classesForCoder() forKey:@"parameters"];
     } @catch (NSException *exception) {
         LOG_ERROR("Failed to decode bundle parameter: %@", exception);
     }
@@ -273,6 +297,8 @@
     if (!parameters)
         return;
 
+    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION([parameters isKindOfClass:[NSDictionary class]]);
+
     if (!m_bundleParameters) {
         m_bundleParameters = adoptNS([[WKWebProcessBundleParameters alloc] initWithDictionary:parameters]);
         return;

Modified: trunk/Tools/ChangeLog (260080 => 260081)


--- trunk/Tools/ChangeLog	2020-04-14 17:17:45 UTC (rev 260080)
+++ trunk/Tools/ChangeLog	2020-04-14 18:07:24 UTC (rev 260081)
@@ -1,3 +1,14 @@
+2020-04-14  Brent Fulgham  <bfulg...@apple.com>
+
+        InjectedBundle parameters often need initialization function called before unarchiving
+        https://bugs.webkit.org/show_bug.cgi?id=189709
+        <rdar://problem/44573653>
+
+        Reviewed by Ryosuke Niwa.
+
+        * TestWebKitAPI/cocoa/WebProcessPlugIn/WebProcessPlugIn.mm:
+        (-[WebProcessPlugIn additionalClassesForParameterCoder]): Added.
+
 2020-04-14  Sergio Villar Senin  <svil...@igalia.com>
 
         [Flatpak] Make run-webkit-tests obey WEBKIT_JHBUILD

Modified: trunk/Tools/TestWebKitAPI/cocoa/WebProcessPlugIn/WebProcessPlugIn.mm (260080 => 260081)


--- trunk/Tools/TestWebKitAPI/cocoa/WebProcessPlugIn/WebProcessPlugIn.mm	2020-04-14 17:17:45 UTC (rev 260080)
+++ trunk/Tools/TestWebKitAPI/cocoa/WebProcessPlugIn/WebProcessPlugIn.mm	2020-04-14 18:07:24 UTC (rev 260081)
@@ -36,6 +36,11 @@
     RetainPtr<id <WKWebProcessPlugIn>> _testPlugIn;
 }
 
+- (NSArray *)additionalClassesForParameterCoder
+{
+    return @[@"MockContentFilterEnabler"];
+}
+
 - (void)webProcessPlugIn:(WKWebProcessPlugInController *)plugInController initializeWithObject:(id)initializationObject
 {
     NSString *testPlugInClassName = [plugInController.parameters valueForKey:TestWebKitAPI::Util::TestPlugInClassNameParameter];
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to