CVE-2018-10895: Fix CSRF issues with qute://settings/set URL
In ffc29ee043
(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
This commit is contained in:
parent
3b9b2bc30e
commit
43e58ac865
@ -32,9 +32,17 @@ import textwrap
|
||||
import mimetypes
|
||||
import urllib
|
||||
import collections
|
||||
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
|
||||
@ -46,6 +54,7 @@ from qutebrowser.qt import sip
|
||||
|
||||
pyeval_output = ":pyeval was never called"
|
||||
spawn_output = ":spawn was never called"
|
||||
csrf_token = None
|
||||
|
||||
|
||||
_HANDLERS = {}
|
||||
@ -449,12 +458,29 @@ 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)
|
||||
|
||||
# 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)
|
||||
confget=config.instance.get_str,
|
||||
csrf_token=csrf_token)
|
||||
return 'text/html', src
|
||||
|
||||
|
||||
|
@ -19,6 +19,7 @@
|
||||
|
||||
"""A request interceptor taking care of adblocking and custom headers."""
|
||||
|
||||
from PyQt5.QtCore import QUrl
|
||||
from PyQt5.QtWebEngineCore import (QWebEngineUrlRequestInterceptor,
|
||||
QWebEngineUrlRequestInfo)
|
||||
|
||||
@ -69,6 +70,18 @@ class RequestInterceptor(QWebEngineUrlRequestInterceptor):
|
||||
resource_type, navigation_type))
|
||||
|
||||
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(url):
|
||||
|
@ -55,8 +55,28 @@ class QuteSchemeHandler(QWebEngineUrlSchemeHandler):
|
||||
job.fail(QWebEngineUrlRequestJob.UrlInvalid)
|
||||
return
|
||||
|
||||
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)
|
||||
|
@ -111,11 +111,13 @@ def dirbrowser_html(path):
|
||||
return html.encode('UTF-8', errors='xmlcharrefreplace')
|
||||
|
||||
|
||||
def handler(request):
|
||||
def handler(request, _operation, _current_url):
|
||||
"""Handler for a file:// URL.
|
||||
|
||||
Args:
|
||||
request: QNetworkRequest to answer to.
|
||||
_operation: The HTTP operation being done.
|
||||
_current_url: The page we're on currently.
|
||||
|
||||
Return:
|
||||
A QNetworkReply for directories, None for files.
|
||||
|
@ -373,13 +373,6 @@ class NetworkManager(QNetworkAccessManager):
|
||||
req, proxy_error, QNetworkReply.UnknownProxyError,
|
||||
self)
|
||||
|
||||
scheme = req.url().scheme()
|
||||
if scheme in self._scheme_handlers:
|
||||
result = self._scheme_handlers[scheme](req)
|
||||
if result is not None:
|
||||
result.setParent(self)
|
||||
return result
|
||||
|
||||
for header, value in shared.custom_headers(url=req.url()):
|
||||
req.setRawHeader(header, value)
|
||||
|
||||
@ -416,5 +409,12 @@ class NetworkManager(QNetworkAccessManager):
|
||||
req.url().toDisplayString(),
|
||||
current_url.toDisplayString()))
|
||||
|
||||
scheme = req.url().scheme()
|
||||
if scheme in self._scheme_handlers:
|
||||
result = self._scheme_handlers[scheme](req, op, current_url)
|
||||
if result is not None:
|
||||
result.setParent(self)
|
||||
return result
|
||||
|
||||
self.set_referer(req, current_url)
|
||||
return super().createRequest(op, req, outgoing_data)
|
||||
|
@ -21,27 +21,46 @@
|
||||
|
||||
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 networkreply
|
||||
from qutebrowser.utils import log, usertypes, qtutils
|
||||
|
||||
|
||||
def handler(request):
|
||||
def handler(request, operation, current_url):
|
||||
"""Scheme handler for qute:// URLs.
|
||||
|
||||
Args:
|
||||
request: QNetworkRequest to answer to.
|
||||
operation: The HTTP operation being done.
|
||||
current_url: The page we're on currently.
|
||||
|
||||
Return:
|
||||
A QNetworkReply.
|
||||
"""
|
||||
if operation != 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)
|
||||
except qutescheme.QuteSchemeOSError as e:
|
||||
|
@ -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);
|
||||
|
20
tests/end2end/data/misc/qutescheme_csrf.html
Normal file
20
tests/end2end/data/misc/qutescheme_csrf.html
Normal file
@ -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>
|
@ -130,6 +130,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
|
||||
|
@ -368,8 +368,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')
|
||||
|
@ -248,7 +248,7 @@ class TestFileSchemeHandler:
|
||||
def test_dir(self, tmpdir):
|
||||
url = QUrl.fromLocalFile(str(tmpdir))
|
||||
req = QNetworkRequest(url)
|
||||
reply = filescheme.handler(req)
|
||||
reply = filescheme.handler(req, None, None)
|
||||
# The URL will always use /, even on Windows - so we force this here
|
||||
# too.
|
||||
tmpdir_path = str(tmpdir).replace(os.sep, '/')
|
||||
@ -259,7 +259,7 @@ class TestFileSchemeHandler:
|
||||
filename.ensure()
|
||||
url = QUrl.fromLocalFile(str(filename))
|
||||
req = QNetworkRequest(url)
|
||||
reply = filescheme.handler(req)
|
||||
reply = filescheme.handler(req, None, None)
|
||||
assert reply is None
|
||||
|
||||
def test_unicode_encode_error(self, mocker):
|
||||
@ -269,5 +269,5 @@ class TestFileSchemeHandler:
|
||||
err = UnicodeEncodeError('ascii', '', 0, 2, 'foo')
|
||||
mocker.patch('os.path.isdir', side_effect=err)
|
||||
|
||||
reply = filescheme.handler(req)
|
||||
reply = filescheme.handler(req, None, None)
|
||||
assert reply is None
|
||||
|
Loading…
Reference in New Issue
Block a user