Skip to content
Permalink
Browse files
Webpages can trigger loads with invalid URLs
https://bugs.webkit.org/show_bug.cgi?id=132224
rdar://problem/16697142

Reviewed by Alexey Proskuryakov.

Invalid URLs can be a way to trick the user about what website they
are looking at.  Still trying to figure out a good way to regression-test this.

* dom/Document.cpp:
(WebCore::Document::processHttpEquiv): Pass a URL rather than a String to
the navigation scheduler.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::receivedFirstData): Ditto.

* loader/NavigationScheduler.cpp:
(WebCore::ScheduledURLNavigation::ScheduledURLNavigation): Take a URL rather
than a string.
(WebCore::ScheduledURLNavigation::url): Ditto.
(WebCore::ScheduledRedirect::ScheduledRedirect): Ditto.
(WebCore::ScheduledLocationChange::ScheduledLocationChange): Ditto.
(WebCore::ScheduledRefresh::ScheduledRefresh): Ditto.
(WebCore::NavigationScheduler::shouldScheduleNavigation): Added a check that
prevents navigation to any URL that is invalid, except for JavaScript URLs,
which need not be valid.
(WebCore::NavigationScheduler::scheduleRedirect): Use URL instead of String.
(WebCore::NavigationScheduler::scheduleLocationChange): Use URL instead of
String. Also got rid of empty string check since empty URLs are also invalid,
and so shouldScheduleNavigation will take care of it.
(WebCore::NavigationScheduler::scheduleRefresh): Use URL instead of String.

* loader/NavigationScheduler.h: Take URL instead of String. Also removed some
unneeded incldues and uses of WTF_MAKE_NONCOPYABLE. NavigationScheduler is
already noncopyable because it has a reference for a data member, and the
disabler doesn't have any real reason to be noncopyable.

* loader/SubframeLoader.cpp:
(WebCore::SubframeLoader::loadOrRedirectSubframe): Pass a URL rather than a
String to the NavigationScheduler.
* page/DOMWindow.cpp:
(WebCore::DOMWindow::createWindow): Ditto.

* page/SecurityOrigin.cpp:
(WebCore::SecurityOrigin::urlWithUniqueSecurityOrigin): Return a URL instead
of a String.
* page/SecurityOrigin.h: Updated for above change.

Canonical link: https://commits.webkit.org/150265@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@167856 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
darinadler committed Apr 27, 2014
1 parent 1314fb9 commit 598d7728c8c640664beff054d3a7e2b85705bdd4
Showing 9 changed files with 97 additions and 47 deletions.
@@ -1,3 +1,52 @@
2014-04-27 Darin Adler <darin@apple.com>

Webpages can trigger loads with invalid URLs
https://bugs.webkit.org/show_bug.cgi?id=132224
rdar://problem/16697142

Reviewed by Alexey Proskuryakov.

Invalid URLs can be a way to trick the user about what website they
are looking at. Still trying to figure out a good way to regression-test this.

* dom/Document.cpp:
(WebCore::Document::processHttpEquiv): Pass a URL rather than a String to
the navigation scheduler.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::receivedFirstData): Ditto.

* loader/NavigationScheduler.cpp:
(WebCore::ScheduledURLNavigation::ScheduledURLNavigation): Take a URL rather
than a string.
(WebCore::ScheduledURLNavigation::url): Ditto.
(WebCore::ScheduledRedirect::ScheduledRedirect): Ditto.
(WebCore::ScheduledLocationChange::ScheduledLocationChange): Ditto.
(WebCore::ScheduledRefresh::ScheduledRefresh): Ditto.
(WebCore::NavigationScheduler::shouldScheduleNavigation): Added a check that
prevents navigation to any URL that is invalid, except for JavaScript URLs,
which need not be valid.
(WebCore::NavigationScheduler::scheduleRedirect): Use URL instead of String.
(WebCore::NavigationScheduler::scheduleLocationChange): Use URL instead of
String. Also got rid of empty string check since empty URLs are also invalid,
and so shouldScheduleNavigation will take care of it.
(WebCore::NavigationScheduler::scheduleRefresh): Use URL instead of String.

* loader/NavigationScheduler.h: Take URL instead of String. Also removed some
unneeded incldues and uses of WTF_MAKE_NONCOPYABLE. NavigationScheduler is
already noncopyable because it has a reference for a data member, and the
disabler doesn't have any real reason to be noncopyable.

* loader/SubframeLoader.cpp:
(WebCore::SubframeLoader::loadOrRedirectSubframe): Pass a URL rather than a
String to the NavigationScheduler.
* page/DOMWindow.cpp:
(WebCore::DOMWindow::createWindow): Ditto.

* page/SecurityOrigin.cpp:
(WebCore::SecurityOrigin::urlWithUniqueSecurityOrigin): Return a URL instead
of a String.
* page/SecurityOrigin.h: Updated for above change.

2014-04-27 Zan Dobersek <zdobersek@igalia.com>

ScriptExecutionContext::Task should work well with C++11 lambdas
@@ -2824,14 +2824,15 @@ void Document::processHttpEquiv(const String& equiv, const String& content)
styleResolverChanged(DeferRecalcStyle);
} else if (equalIgnoringCase(equiv, "refresh")) {
double delay;
String url;
if (frame && parseHTTPRefresh(content, true, delay, url)) {
if (url.isEmpty())
url = m_url.string();
String urlString;
if (frame && parseHTTPRefresh(content, true, delay, urlString)) {
URL completedURL;
if (urlString.isEmpty())
completedURL = m_url;
else
url = completeURL(url).string();
if (!protocolIsJavaScript(url))
frame->navigationScheduler().scheduleRedirect(delay, url);
completedURL = completeURL(urlString);
if (!protocolIsJavaScript(completedURL))
frame->navigationScheduler().scheduleRedirect(delay, completedURL);
else {
String message = "Refused to refresh " + m_url.stringCenterEllipsizedToLength() + " to a javascript: URL";
addConsoleMessage(MessageSource::Security, MessageLevel::Error, message);
@@ -666,16 +666,17 @@ void FrameLoader::receivedFirstData()
return;

double delay;
String url;
if (!parseHTTPRefresh(m_documentLoader->response().httpHeaderField("Refresh"), false, delay, url))
String urlString;
if (!parseHTTPRefresh(m_documentLoader->response().httpHeaderField("Refresh"), false, delay, urlString))
return;
if (url.isEmpty())
url = m_frame.document()->url().string();
URL completedURL;
if (urlString.isEmpty())
completedURL = m_frame.document()->url();
else
url = m_frame.document()->completeURL(url).string();
completedURL = m_frame.document()->completeURL(urlString);

if (!protocolIsJavaScript(url))
m_frame.navigationScheduler().scheduleRedirect(delay, url);
if (!protocolIsJavaScript(completedURL))
m_frame.navigationScheduler().scheduleRedirect(delay, completedURL);
else {
String message = "Refused to refresh " + m_frame.document()->url().stringCenterEllipsizedToLength() + " to a javascript: URL";
m_frame.document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, message);
@@ -97,7 +97,7 @@ class ScheduledNavigation {

class ScheduledURLNavigation : public ScheduledNavigation {
protected:
ScheduledURLNavigation(double delay, SecurityOrigin* securityOrigin, const String& url, const String& referrer, LockHistory lockHistory, LockBackForwardList lockBackForwardList, bool duringLoad, bool isLocationChange)
ScheduledURLNavigation(double delay, SecurityOrigin* securityOrigin, const URL& url, const String& referrer, LockHistory lockHistory, LockBackForwardList lockBackForwardList, bool duringLoad, bool isLocationChange)
: ScheduledNavigation(delay, lockHistory, lockBackForwardList, duringLoad, isLocationChange)
, m_securityOrigin(securityOrigin)
, m_url(url)
@@ -109,7 +109,7 @@ class ScheduledURLNavigation : public ScheduledNavigation {
virtual void fire(Frame& frame) override
{
UserGestureIndicator gestureIndicator(wasUserGesture() ? DefinitelyProcessingUserGesture : DefinitelyNotProcessingUserGesture);
frame.loader().changeLocation(m_securityOrigin.get(), URL(ParsedURLString, m_url), m_referrer, lockHistory(), lockBackForwardList(), false);
frame.loader().changeLocation(m_securityOrigin.get(), m_url, m_referrer, lockHistory(), lockBackForwardList(), false);
}

virtual void didStartTimer(Frame& frame, Timer<NavigationScheduler>& timer) override
@@ -119,7 +119,7 @@ class ScheduledURLNavigation : public ScheduledNavigation {
m_haveToldClient = true;

UserGestureIndicator gestureIndicator(wasUserGesture() ? DefinitelyProcessingUserGesture : DefinitelyNotProcessingUserGesture);
frame.loader().clientRedirected(URL(ParsedURLString, m_url), delay(), currentTime() + timer.nextFireInterval(), lockBackForwardList());
frame.loader().clientRedirected(m_url, delay(), currentTime() + timer.nextFireInterval(), lockBackForwardList());
}

virtual void didStopTimer(Frame& frame, bool newLoadInProgress) override
@@ -137,19 +137,19 @@ class ScheduledURLNavigation : public ScheduledNavigation {
}

SecurityOrigin* securityOrigin() const { return m_securityOrigin.get(); }
String url() const { return m_url; }
const URL& url() const { return m_url; }
String referrer() const { return m_referrer; }

private:
RefPtr<SecurityOrigin> m_securityOrigin;
String m_url;
URL m_url;
String m_referrer;
bool m_haveToldClient;
};

class ScheduledRedirect : public ScheduledURLNavigation {
public:
ScheduledRedirect(double delay, SecurityOrigin* securityOrigin, const String& url, LockHistory lockHistory, LockBackForwardList lockBackForwardList)
ScheduledRedirect(double delay, SecurityOrigin* securityOrigin, const URL& url, LockHistory lockHistory, LockBackForwardList lockBackForwardList)
: ScheduledURLNavigation(delay, securityOrigin, url, String(), lockHistory, lockBackForwardList, false, false)
{
clearUserGesture();
@@ -163,28 +163,28 @@ class ScheduledRedirect : public ScheduledURLNavigation {
virtual void fire(Frame& frame) override
{
UserGestureIndicator gestureIndicator(wasUserGesture() ? DefinitelyProcessingUserGesture : DefinitelyNotProcessingUserGesture);
bool refresh = equalIgnoringFragmentIdentifier(frame.document()->url(), URL(ParsedURLString, url()));
frame.loader().changeLocation(securityOrigin(), URL(ParsedURLString, url()), referrer(), lockHistory(), lockBackForwardList(), refresh);
bool refresh = equalIgnoringFragmentIdentifier(frame.document()->url(), url());
frame.loader().changeLocation(securityOrigin(), url(), referrer(), lockHistory(), lockBackForwardList(), refresh);
}
};

class ScheduledLocationChange : public ScheduledURLNavigation {
public:
ScheduledLocationChange(SecurityOrigin* securityOrigin, const String& url, const String& referrer, LockHistory lockHistory, LockBackForwardList lockBackForwardList, bool duringLoad)
ScheduledLocationChange(SecurityOrigin* securityOrigin, const URL& url, const String& referrer, LockHistory lockHistory, LockBackForwardList lockBackForwardList, bool duringLoad)
: ScheduledURLNavigation(0.0, securityOrigin, url, referrer, lockHistory, lockBackForwardList, duringLoad, true) { }
};

class ScheduledRefresh : public ScheduledURLNavigation {
public:
ScheduledRefresh(SecurityOrigin* securityOrigin, const String& url, const String& referrer)
ScheduledRefresh(SecurityOrigin* securityOrigin, const URL& url, const String& referrer)
: ScheduledURLNavigation(0.0, securityOrigin, url, referrer, LockHistory::Yes, LockBackForwardList::Yes, false, true)
{
}

virtual void fire(Frame& frame) override
{
UserGestureIndicator gestureIndicator(wasUserGesture() ? DefinitelyProcessingUserGesture : DefinitelyNotProcessingUserGesture);
frame.loader().changeLocation(securityOrigin(), URL(ParsedURLString, url()), referrer(), lockHistory(), lockBackForwardList(), true);
frame.loader().changeLocation(securityOrigin(), url(), referrer(), lockHistory(), lockBackForwardList(), true);
}
};

@@ -304,12 +304,18 @@ inline bool NavigationScheduler::shouldScheduleNavigation() const
return m_frame.page();
}

inline bool NavigationScheduler::shouldScheduleNavigation(const String& url) const
inline bool NavigationScheduler::shouldScheduleNavigation(const URL& url) const
{
return shouldScheduleNavigation() && (protocolIsJavaScript(url) || NavigationDisablerForBeforeUnload::isNavigationAllowed());
if (!shouldScheduleNavigation())
return false;
if (protocolIsJavaScript(url))
return true;
if (!url.isValid())
return false;
return NavigationDisablerForBeforeUnload::isNavigationAllowed();
}

void NavigationScheduler::scheduleRedirect(double delay, const String& url)
void NavigationScheduler::scheduleRedirect(double delay, const URL& url)
{
if (!shouldScheduleNavigation(url))
return;
@@ -343,12 +349,10 @@ LockBackForwardList NavigationScheduler::mustLockBackForwardList(Frame& targetFr
return LockBackForwardList::No;
}

void NavigationScheduler::scheduleLocationChange(SecurityOrigin* securityOrigin, const String& url, const String& referrer, LockHistory lockHistory, LockBackForwardList lockBackForwardList)
void NavigationScheduler::scheduleLocationChange(SecurityOrigin* securityOrigin, const URL& url, const String& referrer, LockHistory lockHistory, LockBackForwardList lockBackForwardList)
{
if (!shouldScheduleNavigation(url))
return;
if (url.isEmpty())
return;

if (lockBackForwardList == LockBackForwardList::No)
lockBackForwardList = mustLockBackForwardList(m_frame);
@@ -401,7 +405,7 @@ void NavigationScheduler::scheduleRefresh()
if (url.isEmpty())
return;

schedule(std::make_unique<ScheduledRefresh>(m_frame.document()->securityOrigin(), url.string(), m_frame.loader().outgoingReferrer()));
schedule(std::make_unique<ScheduledRefresh>(m_frame.document()->securityOrigin(), url, m_frame.loader().outgoingReferrer()));
}

void NavigationScheduler::scheduleHistoryNavigation(int steps)
@@ -34,19 +34,16 @@
#include "FrameLoaderTypes.h"
#include "Timer.h"
#include <wtf/Forward.h>
#include <wtf/Noncopyable.h>
#include <wtf/PassRefPtr.h>

namespace WebCore {

class FormSubmission;
class Frame;
class ScheduledNavigation;
class SecurityOrigin;
class URL;

class NavigationDisablerForBeforeUnload {
WTF_MAKE_NONCOPYABLE(NavigationDisablerForBeforeUnload);

public:
NavigationDisablerForBeforeUnload()
{
@@ -64,18 +61,15 @@ class NavigationDisablerForBeforeUnload {
};

class NavigationScheduler {
WTF_MAKE_NONCOPYABLE(NavigationScheduler);

public:
explicit NavigationScheduler(Frame&);
~NavigationScheduler();

bool redirectScheduledDuringLoad();
bool locationChangePending();

void scheduleRedirect(double delay, const String& url);
void scheduleLocationChange(SecurityOrigin*, const String& url, const String& referrer, LockHistory = LockHistory::Yes,
LockBackForwardList = LockBackForwardList::Yes);
void scheduleRedirect(double delay, const URL&);
void scheduleLocationChange(SecurityOrigin*, const URL&, const String& referrer, LockHistory = LockHistory::Yes, LockBackForwardList = LockBackForwardList::Yes);
void scheduleFormSubmission(PassRefPtr<FormSubmission>);
void scheduleRefresh();
void scheduleHistoryNavigation(int steps);
@@ -87,7 +81,7 @@ class NavigationScheduler {

private:
bool shouldScheduleNavigation() const;
bool shouldScheduleNavigation(const String& url) const;
bool shouldScheduleNavigation(const URL&) const;

void timerFired(Timer<NavigationScheduler>&);
void schedule(std::unique_ptr<ScheduledNavigation>);
@@ -324,7 +324,7 @@ Frame* SubframeLoader::loadOrRedirectSubframe(HTMLFrameOwnerElement& ownerElemen
{
Frame* frame = ownerElement.contentFrame();
if (frame)
frame->navigationScheduler().scheduleLocationChange(m_frame.document()->securityOrigin(), url.string(), m_frame.loader().outgoingReferrer(), lockHistory, lockBackForwardList);
frame->navigationScheduler().scheduleLocationChange(m_frame.document()->securityOrigin(), url, m_frame.loader().outgoingReferrer(), lockHistory, lockBackForwardList);
else
frame = loadSubframe(ownerElement, url, frameName, m_frame.loader().outgoingReferrer());

@@ -2018,7 +2018,7 @@ PassRefPtr<Frame> DOMWindow::createWindow(const String& urlString, const AtomicS
newFrame->loader().changeLocation(activeWindow.document()->securityOrigin(), completedURL, referrer, LockHistory::No, LockBackForwardList::No);
else if (!urlString.isEmpty()) {
LockHistory lockHistory = ScriptController::processingUserGesture() ? LockHistory::No : LockHistory::Yes;
newFrame->navigationScheduler().scheduleLocationChange(activeWindow.document()->securityOrigin(), completedURL.string(), referrer, lockHistory, LockBackForwardList::No);
newFrame->navigationScheduler().scheduleLocationChange(activeWindow.document()->securityOrigin(), completedURL, referrer, lockHistory, LockBackForwardList::No);
}

// Navigating the new frame could result in it being detached from its page by a navigation policy delegate.
@@ -36,6 +36,7 @@
#include "SecurityPolicy.h"
#include "ThreadableBlobRegistry.h"
#include <wtf/MainThread.h>
#include <wtf/NeverDestroyed.h>
#include <wtf/StdLibExtras.h>
#include <wtf/text/StringBuilder.h>

@@ -597,10 +598,10 @@ bool SecurityOrigin::isSameSchemeHostPort(const SecurityOrigin* other) const
return true;
}

String SecurityOrigin::urlWithUniqueSecurityOrigin()
URL SecurityOrigin::urlWithUniqueSecurityOrigin()
{
ASSERT(isMainThread());
DEPRECATED_DEFINE_STATIC_LOCAL(const String, uniqueSecurityOriginURL, (ASCIILiteral("data:,")));
static NeverDestroyed<URL> uniqueSecurityOriginURL(ParsedURLString, ASCIILiteral("data:,"));
return uniqueSecurityOriginURL;
}

@@ -205,7 +205,7 @@ class SecurityOrigin : public ThreadSafeRefCounted<SecurityOrigin> {
// (and whether it was set) but considering the host. It is used for postMessage.
bool isSameSchemeHostPort(const SecurityOrigin*) const;

static String urlWithUniqueSecurityOrigin();
static URL urlWithUniqueSecurityOrigin();

private:
SecurityOrigin();

0 comments on commit 598d772

Please sign in to comment.