Bug 323798 - Plastik Decoration broken when the title contains HTML
Summary: Plastik Decoration broken when the title contains HTML
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: decorations (other bugs)
Version First Reported In: 5.6.4
Platform: Neon Linux
: NOR major
Target Milestone: ---
Assignee: KWin default assignee
URL: https://git.reviewboard.kde.org/r/112...
Keywords:
: 327410 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-08-20 19:16 UTC by Alex
Modified: 2016-11-07 10:28 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In: 5.8.4
Sentry Crash Report:
thomas.luebking: ReviewRequest+


Attachments
Correct behaviour (12.77 KB, image/png)
2013-08-20 19:18 UTC, Alex
Details
Broken behaviour (14.37 KB, image/png)
2013-08-20 19:19 UTC, Alex
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alex 2013-08-20 19:16:11 UTC
The Plastik-Decoration zooms the Buttons, when the Title of a window contains "something <div>"

Reproducible: Always

Steps to Reproduce:
1. use Plastik window decoration
2. run kdialog --error "this should not happen" --title "asdf <div>"
Actual Results:  
 window decoration looks broken

Expected Results:  
title should be displayed as is.

This could be security relevant, if some title-strings can do more than just break the decoration.
Comment 1 Alex 2013-08-20 19:18:50 UTC
Created attachment 81816 [details]
Correct behaviour

kdialog --error "this looks nice padding padding padding" --title "asdf"
Comment 2 Alex 2013-08-20 19:19:15 UTC
Created attachment 81817 [details]
Broken behaviour

kdialog --error "this is broken padding padding padding" --title "asdf <div>"
Comment 3 Alex 2013-08-20 19:20:19 UTC
testcase not broken:
kdialog --error "this looks nice padding padding padding" --title "asdf"
Comment 4 Alex 2013-08-20 19:21:38 UTC
changing the title on the same window (testcase: tabs in a webbrowser) changes the title back and forth.
Comment 5 Alex 2013-08-20 19:27:01 UTC
Okay, this is a big fat security issue:
1) open a terminal, run 
$ nc -lp 2000
2) run
$ kdialog --error "you can inject html in KDE titlebars" --title "<img src='http://localhost:2000'>"

expected:
the string as title

result:
> nc -lp 2000
GET / HTTP/1.1
Connection: Keep-Alive
Accept-Encoding: gzip
Accept-Language: de-DE,en,*
User-Agent: Mozilla/5.0
Host: localhost:2000

in the netcat window
Comment 6 Thomas Lübking 2013-08-20 19:55:42 UTC
The patch operates on /usr/share/apps/kwin/decorations/kwin4_decoration_qml_plastik/contents/ui/main.qml

You can inject the line there and restart kwin.
Comment 7 Alex 2013-08-20 21:04:04 UTC
Patch is working. Thank you for the fast reaction.

Maybe you should check other decorations (on my system plastik was the only one with this problem), or write some test, that there is not HTML allowed in parts which are set to untrusted data, like window titles.
Comment 8 Thomas Lübking 2013-08-20 21:08:55 UTC
(In reply to comment #7)

> Maybe you should check other decorations
Aurorae has this flag and Oxygen/Laptop/B2 will not be affected - html interpretation (and even loading external contents) is rather a QML thing while the "traditional" QPainter call just renders strings as they are (+ word wrapping)

Of course external QML driven decos (but not plain Aurorae themes, what you usually get from GHNS) could easily lack that line.


Many thanks for the report, btw.
Comment 9 Martin Flöser 2013-08-21 05:36:58 UTC
(In reply to comment #8)
> Of course external QML driven decos (but not plain Aurorae themes, what you
> usually get from GHNS) could easily lack that line.
We could provide a special Title QML Item which would take care of it - obviously only relevant for the next version (and so far Aurorae is broken like hell)
Comment 10 Thomas Lübking 2013-08-21 19:53:41 UTC
Git commit b10e22b87872ad8d4d1766f2dae780f508f8e2f4 by Thomas Lübking.
Committed on 20/08/2013 at 19:54.
Pushed by luebking into branch 'KDE/4.11'.

make plastik use PlainText captions
FIXED-IN: 4.11
REVIEW: 112180

M  +1    -0    kwin/clients/aurorae/themes/plastik/package/contents/ui/main.qml

http://commits.kde.org/kde-workspace/b10e22b87872ad8d4d1766f2dae780f508f8e2f4
Comment 11 Thomas Lübking 2013-11-10 14:32:47 UTC
*** Bug 327410 has been marked as a duplicate of this bug. ***
Comment 12 Alex 2016-06-21 16:48:45 UTC
The HTML problem seems fixed, but other characters can still break stuff.

Example URL:
http://www.robotinabox.de/was-tun-wenn-schueler-autismus-haben%E2%80%A8%E2%80%A8%E2%80%A8/

xprop output for the title:
WM_ICON_NAME(COMPOUND_TEXT) = "Was tun, wenn Schüler Autismus haben?\342\200\250\342\200\250\342\200\250 | Marlies Hübner - Mozilla Firefox"

affected versions:
- 4.14.2 (debian jessie, plastik decorations)
- 5.6.4 (neon-useredition-20160609-0826, using the defaults)
Comment 13 Martin Flöser 2016-10-28 19:41:30 UTC
The link from comment #12 also breaks with breeze. Given that it would probably be best to fix that in core.
Comment 14 Martin Flöser 2016-10-31 14:54:28 UTC
Git commit 55a0afca97ccfd6b0c3ff3b1516aff36e97c9c89 by Martin Gräßlin.
Committed on 31/10/2016 at 14:54.
Pushed by graesslin into branch 'Plasma/5.8'.

[autotest] Add test case for window caption need to be simplified

The test illustrates that special characters like a line break are not
removed from the window caption, which results in a line break added in
the window decoration.

Test case uses a title from a web page triggering it in Firefox.

M  +1    -0    autotests/integration/CMakeLists.txt
M  +18   -0    autotests/integration/shell_client_test.cpp
A  +131  -0    autotests/integration/x11_client_test.cpp     [License: GPL (v2)]

http://commits.kde.org/kwin/55a0afca97ccfd6b0c3ff3b1516aff36e97c9c89
Comment 15 Martin Flöser 2016-10-31 15:02:05 UTC
Possible patch for the second issue at https://phabricator.kde.org/D3215
Comment 16 Martin Flöser 2016-11-07 10:28:52 UTC
Git commit 2a155925719f7540373386e29c6a23a5c6021b56 by Martin Gräßlin.
Committed on 07/11/2016 at 10:26.
Pushed by graesslin into branch 'Plasma/5.8'.

Simplify the window title passed in from the window system

Summary:
So far KWin used the window title provided from the window directly
without any sanitizing. This could result in broken window decorations
if the title included line breaks. Those were passed to the decoration
and depending on the way how the decoration renders the title, it could
result in visual breakage.

Having line breaks in a window title doesn't make sense. Given that KWin
now simplifies the title when copying it to it's own structure. This
also ensures that the title passed to e.g. task manager does not have
any line breaks on Wayland.
FIXED-IN: 5.8.4

Test Plan: Opened the web page in a nested KWin, properly rendered now.

Reviewers: #kwin, #plasma

Subscribers: plasma-devel, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D3215

M  +0    -2    autotests/integration/shell_client_test.cpp
M  +0    -2    autotests/integration/x11_client_test.cpp
M  +2    -2    client.cpp
M  +2    -2    shell_client.cpp

http://commits.kde.org/kwin/2a155925719f7540373386e29c6a23a5c6021b56