Permalink
Browse files

CVE-2018-10895: Fix CSRF issues with qute://settings/set URL

In ffc29ee (part of v1.0.0), a
qute://settings/set URL was added to change settings.

Contrary to what I apparently believed at the time, it *is* possible for
websites to access `qute://*` URLs (i.e., neither QtWebKit nor QtWebEngine
prohibit such requests, other than the usual cross-origin rules).

In other words, this means a website can e.g. have an `<img>` tag which loads a
`qute://settings/set` URL, which then sets `editor.command` to a bash script.
The result of that is arbitrary code execution.

Fixes #4060
See #2332

(cherry picked from commit 43e58ac)
  • Loading branch information...
The-Compiler committed Jul 9, 2018
1 parent 8486efa commit ff686ff7f395d83e5ac48507ecfae0b0e97a61ef
@@ -30,9 +30,17 @@
import textwrap
import mimetypes
import urllib
import base64
try:
import secrets
except ImportError:
# New in Python 3.6
secrets = None
import pkg_resources
from PyQt5.QtCore import QUrlQuery, QUrl
from PyQt5.QtNetwork import QNetworkReply
import qutebrowser
from qutebrowser.config import config, configdata, configexc, configdiff
@@ -43,6 +51,7 @@
pyeval_output = ":pyeval was never called"
spawn_output = ":spawn was never called"
csrf_token = None
_HANDLERS = {}
@@ -426,13 +435,30 @@ def _qute_settings_set(url):
@add_handler('settings')
def qute_settings(url):
"""Handler for qute://settings. View/change qute configuration."""
global csrf_token
if url.path() == '/set':
if url.password() != csrf_token:
message.error("Invalid CSRF token for qute://settings!")
raise QuteSchemeError("Invalid CSRF token!",
QNetworkReply.ContentAccessDenied)
return _qute_settings_set(url)
html = jinja.render('settings.html', title='settings',
configdata=configdata,
confget=config.instance.get_str)
return 'text/html', html
# Requests to qute://settings/set should only be allowed from
# qute://settings. As an additional security precaution, we generate a CSRF
# token to use here.
if secrets:
csrf_token = secrets.token_urlsafe()
else:
# On Python < 3.6, from secrets.py
token = base64.urlsafe_b64encode(os.urandom(32))
csrf_token = token.rstrip(b'=').decode('ascii')
src = jinja.render('settings.html', title='settings',
configdata=configdata,
confget=config.instance.get_str,
csrf_token=csrf_token)
return 'text/html', src
@add_handler('back')
@@ -19,7 +19,9 @@
"""A request interceptor taking care of adblocking and custom headers."""
from PyQt5.QtWebEngineCore import QWebEngineUrlRequestInterceptor
from PyQt5.QtCore import QUrl
from PyQt5.QtWebEngineCore import (QWebEngineUrlRequestInterceptor,
QWebEngineUrlRequestInfo)
from qutebrowser.config import config
from qutebrowser.browser import shared
@@ -54,6 +56,20 @@ def interceptRequest(self, info):
Args:
info: QWebEngineUrlRequestInfo &info
"""
url = info.requestUrl()
firstparty = info.firstPartyUrl()
if ((url.scheme(), url.host(), url.path()) ==
('qute', 'settings', '/set')):
if (firstparty != QUrl('qute://settings/') or
info.resourceType() !=
QWebEngineUrlRequestInfo.ResourceTypeXhr):
log.webview.warning("Blocking malicious request from {} to {}"
.format(firstparty.toDisplayString(),
url.toDisplayString()))
info.block(True)
return
# FIXME:qtwebengine only block ads for NavigationTypeOther?
if self._host_blocker.is_blocked(info.requestUrl()):
log.webview.info("Request to {} blocked by host blocker.".format(
@@ -45,8 +45,29 @@ def requestStarted(self, job):
job: QWebEngineUrlRequestJob
"""
url = job.requestUrl()
assert job.requestMethod() == b'GET'
# Only the browser itself or qute:// pages should access any of those
# URLs.
# The request interceptor further locks down qute://settings/set.
try:
initiator = job.initiator()
except AttributeError:
# Added in Qt 5.11
pass
else:
if initiator.isValid() and initiator.scheme() != 'qute':
log.misc.warning("Blocking malicious request from {} to {}"
.format(initiator.toDisplayString(),
url.toDisplayString()))
job.fail(QWebEngineUrlRequestJob.RequestDenied)
return
if job.requestMethod() != b'GET':
job.fail(QWebEngineUrlRequestJob.RequestDenied)
return
assert url.scheme() == 'qute'
log.misc.debug("Got request for {}".format(url.toDisplayString()))
try:
mimetype, data = qutescheme.data_for_url(url)
@@ -115,13 +115,14 @@ class FileSchemeHandler(schemehandler.SchemeHandler):
"""Scheme handler for file: URLs."""
def createRequest(self, _op, request, _outgoing_data):
def createRequest(self, _op, request, _outgoing_data, _current_url):
"""Create a new request.
Args:
request: const QNetworkRequest & req
_op: Operation op
_outgoing_data: QIODevice * outgoingData
_current_url: The URL currently being visited.
Return:
A QNetworkReply for directories, None for files.
@@ -384,13 +384,6 @@ def createRequest(self, op, req, outgoing_data):
req, proxy_error, QNetworkReply.UnknownProxyError,
self)
scheme = req.url().scheme()
if scheme in self._scheme_handlers:
result = self._scheme_handlers[scheme].createRequest(
op, req, outgoing_data)
if result is not None:
return result
for header, value in shared.custom_headers():
req.setRawHeader(header, value)
@@ -418,5 +411,12 @@ def createRequest(self, op, req, outgoing_data):
# the webpage shutdown here.
current_url = QUrl()
scheme = req.url().scheme()
if scheme in self._scheme_handlers:
result = self._scheme_handlers[scheme].createRequest(
op, req, outgoing_data, current_url)
if result is not None:
return result
self.set_referer(req, current_url)
return super().createRequest(op, req, outgoing_data)
@@ -21,7 +21,8 @@
import mimetypes
from PyQt5.QtNetwork import QNetworkReply
from PyQt5.QtCore import QUrl
from PyQt5.QtNetwork import QNetworkReply, QNetworkAccessManager
from qutebrowser.browser import pdfjs, qutescheme
from qutebrowser.browser.webkit.network import schemehandler, networkreply
@@ -32,22 +33,39 @@ class QuteSchemeHandler(schemehandler.SchemeHandler):
"""Scheme handler for qute:// URLs."""
def createRequest(self, _op, request, _outgoing_data):
def createRequest(self, op, request, _outgoing_data, current_url):
"""Create a new request.
Args:
request: const QNetworkRequest & req
_op: Operation op
op: Operation op
_outgoing_data: QIODevice * outgoingData
current_url: The page we're on currently.
Return:
A QNetworkReply.
"""
if op != QNetworkAccessManager.GetOperation:
return networkreply.ErrorNetworkReply(
request, "Unsupported request type",
QNetworkReply.ContentOperationNotPermittedError)
url = request.url()
if ((url.scheme(), url.host(), url.path()) ==
('qute', 'settings', '/set')):
if current_url != QUrl('qute://settings/'):
log.webview.warning("Blocking malicious request from {} to {}"
.format(current_url.toDisplayString(),
url.toDisplayString()))
return networkreply.ErrorNetworkReply(
request, "Invalid qute://settings request",
QNetworkReply.ContentAccessDenied)
try:
mimetype, data = qutescheme.data_for_url(request.url())
mimetype, data = qutescheme.data_for_url(url)
except qutescheme.NoHandlerFound:
errorstr = "No handler found for {}!".format(
request.url().toDisplayString())
errorstr = "No handler found for {}!".format(url.toDisplayString())
return networkreply.ErrorNetworkReply(
request, errorstr, QNetworkReply.ContentNotFoundError,
self.parent())
@@ -3,7 +3,8 @@
{% block script %}
var cset = function(option, value) {
// FIXME:conf we might want some error handling here?
var url = "qute://settings/set?option=" + encodeURIComponent(option);
var url = "qute://user:{{csrf_token}}@settings/set"
url += "?option=" + encodeURIComponent(option);
url += "&value=" + encodeURIComponent(value);
var xhr = new XMLHttpRequest();
xhr.open("GET", url);
@@ -0,0 +1,20 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>CSRF issues with qute://settings</title>
<script type="text/javascript">
function add_img() {
const elem = document.createElement("img")
elem.src = "qute://settings/set?option=auto_save.interval&value=invalid";
document.body.appendChild(elem);
}
</script>
</head>
<body>
<form action="qute://settings/set?option=auto_save.interval&value=invalid" method="post"><button type="submit" id="via-form">Via form</button></form>
<input type="button" onclick="add_img()" value="Via img" id="via-img">
<a href="qute://settings/set?option=auto_save.interval&value=invalid" id="via-link">Via link</a>
<a href="/redirect-to?url=qute://settings/set%3Foption=auto_save.interval%26value=invalid" id="via-redirect">Via redirect</a>
</body>
</html>
@@ -128,6 +128,63 @@ Feature: Special qute:// pages
And I press the key "<Tab>"
Then "Invalid value 'foo' *" should be logged
@qtwebkit_skip
Scenario: qute://settings CSRF via img (webengine)
When I open data/misc/qutescheme_csrf.html
And I run :click-element id via-img
Then "Blocking malicious request from http://localhost:*/data/misc/qutescheme_csrf.html to qute://settings/set?*" should be logged
@qtwebkit_skip
Scenario: qute://settings CSRF via link (webengine)
When I open data/misc/qutescheme_csrf.html
And I run :click-element id via-link
Then "Blocking malicious request from qute://settings/set?* to qute://settings/set?*" should be logged
@qtwebkit_skip
Scenario: qute://settings CSRF via redirect (webengine)
When I open data/misc/qutescheme_csrf.html
And I run :click-element id via-redirect
Then "Blocking malicious request from qute://settings/set?* to qute://settings/set?*" should be logged
@qtwebkit_skip
Scenario: qute://settings CSRF via form (webengine)
When I open data/misc/qutescheme_csrf.html
And I run :click-element id via-form
Then "Blocking malicious request from qute://settings/set?* to qute://settings/set?*" should be logged
@qtwebkit_skip
Scenario: qute://settings CSRF token (webengine)
When I open qute://settings
And I run :jseval const xhr = new XMLHttpRequest(); xhr.open("GET", "qute://settings/set"); xhr.send()
Then "Error while handling qute://* URL" should be logged
And the error "Invalid CSRF token for qute://settings!" should be shown
@qtwebengine_skip
Scenario: qute://settings CSRF via img (webkit)
When I open data/misc/qutescheme_csrf.html
And I run :click-element id via-img
Then "Blocking malicious request from http://localhost:*/data/misc/qutescheme_csrf.html to qute://settings/set?*" should be logged
@qtwebengine_skip
Scenario: qute://settings CSRF via link (webkit)
When I open data/misc/qutescheme_csrf.html
And I run :click-element id via-link
Then "Blocking malicious request from http://localhost:*/data/misc/qutescheme_csrf.html to qute://settings/set?*" should be logged
And "Error while loading qute://settings/set?*: Invalid qute://settings request" should be logged
@qtwebengine_skip
Scenario: qute://settings CSRF via redirect (webkit)
When I open data/misc/qutescheme_csrf.html
And I run :click-element id via-redirect
Then "Blocking malicious request from http://localhost:*/data/misc/qutescheme_csrf.html to qute://settings/set?*" should be logged
And "Error while loading qute://settings/set?*: Invalid qute://settings request" should be logged
@qtwebengine_skip
Scenario: qute://settings CSRF via form (webkit)
When I open data/misc/qutescheme_csrf.html
And I run :click-element id via-form
Then "Error while loading qute://settings/set?*: Unsupported request type" should be logged
# pdfjs support
@qtwebengine_skip: pdfjs is not implemented yet
@@ -358,8 +358,10 @@ def test_qute_settings_persistence(short_tmpdir, request, quteproc_new):
"""Make sure settings from qute://settings are persistent."""
args = _base_args(request.config) + ['--basedir', str(short_tmpdir)]
quteproc_new.start(args)
quteproc_new.open_path(
'qute://settings/set?option=search.ignore_case&value=always')
quteproc_new.open_path('qute://settings/')
quteproc_new.send_cmd(':jseval --world main '
'cset("search.ignore_case", "always")')
assert quteproc_new.get_setting('search.ignore_case') == 'always'
quteproc_new.send_cmd(':quit')
@@ -249,7 +249,7 @@ def test_dir(self, tmpdir):
url = QUrl.fromLocalFile(str(tmpdir))
req = QNetworkRequest(url)
handler = filescheme.FileSchemeHandler(win_id=0)
reply = handler.createRequest(None, req, None)
reply = handler.createRequest(None, req, None, None)
# The URL will always use /, even on Windows - so we force this here
# too.
tmpdir_path = str(tmpdir).replace(os.sep, '/')
@@ -261,7 +261,7 @@ def test_file(self, tmpdir):
url = QUrl.fromLocalFile(str(filename))
req = QNetworkRequest(url)
handler = filescheme.FileSchemeHandler(win_id=0)
reply = handler.createRequest(None, req, None)
reply = handler.createRequest(None, req, None, None)
assert reply is None
def test_unicode_encode_error(self, mocker):
@@ -272,5 +272,5 @@ def test_unicode_encode_error(self, mocker):
err = UnicodeEncodeError('ascii', '', 0, 2, 'foo')
mocker.patch('os.path.isdir', side_effect=err)
reply = handler.createRequest(None, req, None)
reply = handler.createRequest(None, req, None, None)
assert reply is None

0 comments on commit ff686ff

Please sign in to comment.