Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

History page parses and renders html tags #4011

Closed
BenjaminBrandtner opened this issue Jun 21, 2018 · 8 comments
Closed

History page parses and renders html tags #4011

BenjaminBrandtner opened this issue Jun 21, 2018 · 8 comments
Labels
bug: behavior Something doesn't work as intended, but doesn't crash. priority: 0 - high Issues which are currently the primary focus.
Milestone

Comments

@BenjaminBrandtner
Copy link

BenjaminBrandtner commented Jun 21, 2018

While visiting this page which has an html input element as it's title (<input type="file">), I noticed that it got parsed like HTML and actually displays the rendered element in the qute://history/ page, whereas expected/desired behavoir would be the website title being displayed in plain text.

I then searched a bunch of unclosed html tags in google, a <b> and an <h1>. The 3 topmost items in this screenshot show that it all gets evaluated:

screenshot 11

Version Information:
http://paste.the-compiler.org/view/61009fa1

@The-Compiler
Copy link
Member

The-Compiler commented Jun 21, 2018

Whoops... 😊 That also means it's possible for pages to do XSS via the page title when an user visits the history page, which presumably at least allows them to read the user's browsing history.

cc @imransobir

@The-Compiler The-Compiler added priority: 0 - high Issues which are currently the primary focus. bug: behavior Something doesn't work as intended, but doesn't crash. labels Jun 21, 2018
@The-Compiler The-Compiler added this to the v1.4.0 milestone Jun 21, 2018
@The-Compiler
Copy link
Member

After taking a closer look, I think it's not possible to inject <script> tags - or rather, they don't get executed (because the element is created dynamically via JavaScript).

@The-Compiler
Copy link
Member

Welp, it is actually possible - see the "security considerations" for innerHTML.

The-Compiler added a commit that referenced this issue Jun 21, 2018
Fixes #4011

(cherry picked from commit 5a7869f)
@The-Compiler
Copy link
Member

I requested a CVE for this issue, see https://pending-requests-v5.distributedweaknessfiling.org/

@The-Compiler
Copy link
Member

Here's a copy of the announcement I sent out:


Hey,

I've just released qutebrowser v1.3.3, which fixes an XSS vulnerability
on the qute://history page (:history).

The vulnerability allowed websites to inject HTML into the page via a
crafted title tag. This could allow them to steal your browsing history.

If you're currently unable to upgrade, avoid using :history.

A CVE request for this issue is pending, I'll send out another mail once
there's a CVE ID assigned.

The issue was introduced in March 2017 and part of the v0.11.0 release:
845f21b
1179ee7

The patch applies cleanly to v1.2.x and v1.1.x (but I do not plan to do
any updated releases of those):
https://github.com/qutebrowser/qutebrowser/commit/5a7869f2feaa346853d2a85413d6527c87ef0d9f.patch

It does not apply to v1.0.x and v0.11.x. If you need a backport,
please let me know, but especially on v0.11.x you'll probably have a lot
of other security issues due to an outdated QtWebKit anyways.

I plan to release v1.4.0 later this week (once PyQt 5.11 is out), but
since the bug was opened publicly, I decided to do an immediate bugfix
release. As a reminder, for security-relevant bugs, please contact me
directly at mail at qutebrowser.org.

Other bugfixes in this release:

  • Crash in a workaround for a Qt 5.11 bug in rare circumstances.
  • Workaround for a Qt bug which preserves searches between page loads.
  • In v1.3.2 a dependency on the PyQt5.QtQuickWidgets module was accidentally
    introduced. Since that module isn't packaged everywhere, it's been removed
    again.

Sorry for the trouble!

Florian

@The-Compiler
Copy link
Member

Unfortunately my fix caused a regression due to escaping URLs as well, causing the wrong URL to be loaded when clicking a link on the history page. I'm tracking this in #4012. I won't release a v1.3.4 because this is rather minor, but I plan to fix it in v1.4.0.

@BenjaminBrandtner
Copy link
Author

BenjaminBrandtner commented Jun 22, 2018

I'm glad my first ever contribution to an open source project was useful and able to expose and fix a rather important bug. Here's to many more! (Contributions, not bugs) :)

The-Compiler added a commit that referenced this issue Jun 24, 2018
We only use the URL to set a 'href' attribute, which does not need escaping.

See #4011
Fixes #4012
0-wiz-0 pushed a commit to NetBSD/pkgsrc-wip that referenced this issue Jun 25, 2018
v1.3.3
------

Security
~~~~~~~~

- An XSS vulnerability on the `qute://history` page allowed websites to inject
  HTML into the page via a crafted title tag. This could allow them to steal
  your browsing history. If you're currently unable to upgrade, avoid using
  `:history`. A CVE request for this issue is pending, see
  qutebrowser/qutebrowser#4011 for updates.

Fixed
~~~~~

- Crash in a workaround for a Qt 5.11 bug in rare circumstances.
- Workaround for a Qt bug which preserves searches between page loads.
- In v1.3.2 a dependency on the `PyQt5.QtQuickWidgets` module was accidentally
  introduced. Since that module isn't packaged everywhere, it's been removed
  again.

v1.3.2
------

Fixed
~~~~~

- QtWebEngine: Improved workaround for a bug in Qt 5.11 where only the
  top/bottom half of the window is used.
- QtWebEngine: Work around a bug in Qt 5.11 where an endless loading-loop is
  triggered when clicking a link with an unknown scheme.
- QtWebEngine: When switching between pages with changed settings, less
  unnecessary reloads are done now.
- QtWebEngine: It's now possible to open external links such as `magnet://` or
  `mailto:` via hints.
@The-Compiler
Copy link
Member

This issue has been assigned CVE-2018-1000559.

@BenjaminBrandtner Thanks again, especially for the screenshots and all which makes things easier! I'm not sure if you were aware of the security implications of this bug (and it's okay if you weren't!) - but if you were, note that there's usually a way to report security related issues to projects privately (mail@qutebrowser.org for qutebrowser). That way, there's less pressure on maintainers to fix things immediately, as things are less likely to be exploited (and it's possible to only announce the details when distributions updated their packages).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: behavior Something doesn't work as intended, but doesn't crash. priority: 0 - high Issues which are currently the primary focus.
Projects
None yet
Development

No branches or pull requests

2 participants