We shouldn't let local files access web URLs. Our current behavior makes HTML files loaded from your local file system super dangerous.
Created attachment 35180 [details] Patch v1
Chrome has been running with this setting for like a year with no problems.
Comment on attachment 35180 [details] Patch v1 > diff --git a/WebKit/mac/ChangeLog b/WebKit/mac/ChangeLog > index 5335efb..e01f625 100644 > --- a/WebKit/mac/ChangeLog > +++ b/WebKit/mac/ChangeLog > @@ -1,3 +1,13 @@ > +2009-08-19 Adam Barth <abarth@webkit.org> > + > + Reviewed by NOBODY (OOPS!). > + > + Don't let local files access web URLs > + https://bugs.webkit.org/show_bug.cgi?id=28480 > + > + * WebView/WebPreferences.mm: > + (+[WebPreferences initialize]): > + > 2009-08-18 Anders Carlsson <andersca@apple.com> > > Reviewed by Adele Peterson. > diff --git a/WebKit/mac/WebView/WebPreferences.mm b/WebKit/mac/WebView/WebPreferences.mm > index 0b40b59..cfd24ef 100644 > --- a/WebKit/mac/WebView/WebPreferences.mm > +++ b/WebKit/mac/WebView/WebPreferences.mm > @@ -315,7 +315,11 @@ static WebCacheModel cacheModelForMainBundle(void) > [NSNumber numberWithBool:YES], WebKitJavaEnabledPreferenceKey, > [NSNumber numberWithBool:YES], WebKitJavaScriptEnabledPreferenceKey, > [NSNumber numberWithBool:YES], WebKitWebSecurityEnabledPreferenceKey, > +#if ENABLE(DASHBOARD_SUPPORT) > [NSNumber numberWithBool:YES], WebKitAllowUniversalAccessFromFileURLsPreferenceKey, > +#else > + [NSNumber numberWithBool:NO], WebKitAllowUniversalAccessFromFileURLsPreferenceKey, > +#endif > [NSNumber numberWithBool:YES], WebKitJavaScriptCanOpenWindowsAutomaticallyPreferenceKey, > [NSNumber numberWithBool:YES], WebKitPluginsEnabledPreferenceKey, > [NSNumber numberWithBool:YES], WebKitDatabasesEnabledPreferenceKey, Since dashboard support is always compiled in on Mac OS X this still results in the setting being on for Mac OS X. A more sensible change may be to use a linked-on-or-after check to set default value of this preference to YES for existing applications while newly-written applications would get NO as the value. You also appear to have omitted GTK from these changes.
Created attachment 38357 [details] set file url pref to secure --- 11 files changed, 61 insertions(+), 3 deletions(-)
Comment on attachment 38357 [details] set file url pref to secure Looks fine to me. What about Gtk? Mark should at least see this since he reviewed the previous patch.
I'm going to hold off landing this for a little while to give folks a chance to comment. In the blackout, I commented that I omitted GTK on purpose because I didn't see a public API for manipulating this setting. I'll file a followup bug about that after we resolve this one.
(In reply to comment #6) > I'm going to hold off landing this for a little while to give folks a chance to > comment. > > In the blackout, I commented that I omitted GTK on purpose because I didn't see > a public API for manipulating this setting. I'll file a followup bug about > that after we resolve this one. FWIW, for GTK, we have WebKit/gtk/webkit/webkitwebsettings for the API and update the values of WebCore::Settings in WebKit/gtk/webkit/webkitwebview (webkit_web_view_update_settings function).
- CFDictionaryAddValue(defaults, CFSTR(WebKitAllowUniversalAccessFromFileURLsPreferenceKey), kCFBooleanTrue); + CFDictionaryAddValue(defaults, CFSTR(WebKitAllowUniversalAccessFromFileURLsPreferenceKey), kCFBooleanFalse); These are default preferences - what about users who already had Safari installed on their machines before upgrading to a version that has this change?
(In reply to comment #8) > These are default preferences - what about users who already had Safari > installed on their machines before upgrading to a version that has this change? Oh, I have no idea how preferences work on the Mac. If they haven't overridden the default, will this not affect them? I guess one option is to rename the pref key.
Hmm, you are probably right - I don't see the preference written in com.apple.Safari defaults, so the default doesn't seem to be permanent.
Comment on attachment 38357 [details] set file url pref to secure Rejecting patch 38357 from commit-queue. This patch will require manual commit. Found no modified ChangeLogs, cannot create a commit message. All changes require a ChangeLog. See: http://webkit.org/coding/contributing.html
Comment on attachment 38357 [details] set file url pref to secure Clearing flags on attachment: 38357 Committed r47684: <http://trac.webkit.org/changeset/47684>
All reviewed patches have been landed. Closing bug.
After a discussion on IRC, I've reverted this change. The consensus was to make this change later after various clients have had a chance to opt into the insecure setting if they need it for some reason.
Created attachment 38528 [details] set file url pref to secure --- 8 files changed, 44 insertions(+), 2 deletions(-)
As discussed on webkit-dev, I've updated this patch to affect win and qt, but not mac. I'd still like to do GTK in another patch because I don't quite understand its preference system.
(In reply to comment #16) > not mac. I'd still like to do GTK in another patch because I don't quite > understand its preference system. Hi Adam. This is already implemented for GTK[1] following your email to webkit-dev. [1] http://trac.webkit.org/changeset/47690
(In reply to comment #17) > This is already implemented for GTK[1] following your email to webkit-dev. Awesome! One detail: you might want to set the setting to true in DumpRenderTree because some of the LayoutTests assume file URLs can access data URLs.
Comment on attachment 38528 [details] set file url pref to secure Rejecting patch 38528 from commit-queue. This patch will require manual commit. ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1
Comment on attachment 38528 [details] set file url pref to secure Clearing flags on attachment: 38528 Committed r47807: <http://trac.webkit.org/changeset/47807>
Sorry. run-webkit-tests had left an httpd process running causing all http tests to fail on the commit-queue bot.