WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
156009
run-webkit-tests must create parent directory of user's cache directory before running tests
https://bugs.webkit.org/show_bug.cgi?id=156009
Summary
run-webkit-tests must create parent directory of user's cache directory befor...
Daniel Bates
Reported
2016-03-29 22:08:13 PDT
The script run-webkit-tests influences the path chosen by a WebKit2 child process for the user's temporary directory and user's cache directory via the environment variable DIRHELPER_USER_DIR_SUFFIX. For the user's temporary directory, run-webkit-tests creates it as part of setting up the test environment. But run-webkit-tests does not create the user's cache directory. Therefore there is race between the time AppKit creates it and when the launched WebContent and Network processes query for the path to the user's cache directory such that the OS may return the empty string to the WebContent and Network processes if queried for this directory before AppKit creates it.
Attachments
Patch
(7.24 KB, patch)
2016-03-29 22:11 PDT
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Patch
(7.35 KB, patch)
2016-03-29 22:14 PDT
,
Daniel Bates
ap
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Daniel Bates
Comment 1
2016-03-29 22:11:42 PDT
Created
attachment 275182
[details]
Patch
Daniel Bates
Comment 2
2016-03-29 22:14:26 PDT
Created
attachment 275183
[details]
Patch
Alexey Proskuryakov
Comment 3
2016-03-29 23:54:58 PDT
Comment on
attachment 275183
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=275183&action=review
> Tools/Scripts/webkitpy/port/driver.py:136 > + self._driver_user_directory_suffix = None > + self._driver_user_cache_directory = None
Maybe we should include the user directory for completeness ("/0/").
Daniel Bates
Comment 4
2016-03-30 09:26:33 PDT
(In reply to
comment #3
)
> > Tools/Scripts/webkitpy/port/driver.py:136 > > + self._driver_user_directory_suffix = None > > + self._driver_user_cache_directory = None > > Maybe we should include the user directory for completeness ("/0/").
Filed
bug #156025
to teach run-webkit-test to create and delete the directory _CS_DARWIN_USER_DIR.
Daniel Bates
Comment 5
2016-03-30 10:01:07 PDT
(In reply to
comment #0
)
> [...] the OS may return the empty string to the WebContent and Network processes if queried for [the user's cache] directory before AppKit creates it.
Further elaborating on this, the WebContent (Network) process query the OS for the path to the user's cache directory with DIRHELPER_USER_DIR_SUFFIX of the form WebKitTestRunner-X/com.apple.WebKit.WebContent.Development (WebKitTestRunner-X/com.apple.WebKit.Networking.Development) by [1]. The processes query the OS via confstr(_CS_DARWIN_USER_CACHE_DIR, ..., ...). And confstr(_CS_DARWIN_USER_CACHE_DIR, ..., ...) will create the directory if it does not exist. At the time of writing, confstr(_CS_DARWIN_USER_CACHE_DIR, ..., ...) returns a path of the form $TMPDIR/C/$DIRHELPER_USER_DIR_SUFFIX where $TMPDIR and $DIRHELPER_USER_DIR_SUFFIX are the values of the environment variables of the same name. When DIRHELPER_USER_DIR_SUFFIX := WebKitTestRunner-X/com.apple.WebKit.WebContent.Development, confstr(_CS_DARWIN_USER_CACHE_DIR, ..., ...) will compute the path $TMPDIR/C/WebKitTestRunner-X/com.apple.WebKit.WebContent.Development and call mkdir() on it. Notice that mkdir() does not recursively create directory hierarchies. If $TMPDIR/C/WebKitTestRunner-X does not exist (because AppKit did not create it, yet) then mkdir($TMPDIR/C/WebKitTestRunner-X/com.apple.WebKit.WebContent.Development) will fail with errno ENOENT - "A component of the path prefix does not exist or path is an empty string." (MKDIR(2)) This failure causes a sandbox initialization error in the NetworkProcess by <
http://trac.webkit.org/browser/trunk/Source/WebKit2/NetworkProcess/mac/com.apple.WebKit.NetworkProcess.sb.in?rev=197808#L77
> because the empty string will be passed for parameter DARWIN_USER_CACHE_DIR. [1] Disregarding the fact that the code has a correctness issue that prevents it from ever making use of such a suffix (we will fix this in
bug #155455
), the code that builds this DIRHELPER_USER_DIR_SUFFIX is at <
http://trac.webkit.org/browser/trunk/Source/WebKit2/Shared/mac/ChildProcessMac.mm?rev=198237#L131
>.
Daniel Bates
Comment 6
2016-03-30 10:18:47 PDT
For completeness, Alexey Proskuryakov and I decided to fix this issue by teaching run-webkit-tests to create the user's cache directory instead of teaching the WebKit2 child process to create it to avoid adding filesystem code to WebKit2 with respect to the use of the value of the environment variable DIRHELPER_USER_DIR_SUFFIX. The value of this variable should never be used outside of WebKit's test infrastructure as it has security implications. We only make use of DIRHELPER_USER_DIR_SUFFIX when the UI process (client app) is not sandboxed as is the case for DumpRenderTree/WebKitTestRunner by <
http://trac.webkit.org/browser/trunk/Source/WebKit2/Shared/EntryPointUtilities/mac/XPCService/XPCServiceEntryPoint.mm?rev=190257#L100
>. We felt that adding code to WebKit2 to create the user's cache directory based on DIRHELPER_USER_DIR_SUFFIX could increase the attack surface should the WebKit2 be refactored without consideration of the security implications of DIRHELPER_USER_DIR_SUFFIX.
Radar WebKit Bug Importer
Comment 7
2016-03-30 10:21:29 PDT
<
rdar://problem/25442682
>
Daniel Bates
Comment 8
2016-03-30 10:23:41 PDT
Committed
r198842
: <
http://trac.webkit.org/changeset/198842
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug