Version: unspecified (using KDE 4.7.4) OS: Linux See below. Reproducible: Always Steps to Reproduce: 0. have a program with window title with some html 1. make the program to hang (while (1) do something) 2. try to close it and wait for the termination warning dialog Actual Results: html is rendered (see the screenshot) Expected Results: html is not rendered
Created attachment 68629 [details] Example of the issue
Do you have a test case for this issue?
Created attachment 70330 [details] Reproducible example Yes, this is a short example that creates a window and when you'll try to close freezes. Trying to close it multiple times will reproduce the issue. To compile (Qt is needed): qmake make
Created attachment 70331 [details] Proposed patch Proposed patch, I didn't try it, so might even not compile. best Marco Cimmino
KDE 4.8.2 also affected. Any other things you need to fix this bug??
I guess by comment #2 Martin either wondered whether - you've an exploit where this causes some vulnerability (you have not because qlabel can handle html, but "injection" sounds like) or - there's an actual client using html tags in the title (ie. you've a special interest) because otherwise this "bug" - only affects text rendering - in the itself rare case of a hanging client (a *buggy* client) - which probably doesn't exist anyway and is by this rather theoretic what raises the question whether the fix of this looooong standing "issue" justifies even remotely risking a regression in a minor release.
btw patches on bug reports get easily lost. You might want to propose it on reviewboard.kde.org
(In reply to comment #6) > I guess by comment #2 > Martin either wondered whether > - you've an exploit where this causes some vulnerability (you have not > because qlabel can handle html, but "injection" sounds like) or > - there's an actual client using html tags in the title (ie. you've a > special interest) because > > otherwise this "bug" > - only affects text rendering > - in the itself rare case of a hanging client (a *buggy* client) > - which probably doesn't exist anyway > > and is by this rather theoretic what raises the question whether the fix of > this looooong standing "issue" justifies even remotely risking a regression > in a minor release. Developers that spend more time finding excuses to not fix a bug rather than fixing it they are funny to me. Especially when there is a proposed patch already that needs just to be reviewed. Really do you think that a couple of escaped description strings can 'risking a regression'? Funny, just funny.
But I understand your point, you don't fix even more important issues leaving non working features across different major versions, this bug from your point of view is not even a bug. I apologize if I tried to make KDE a bit better.
Marco, I can see you are frustrated. Thank you for your patch. Please understand that the KWin team is probably one of the ones in KDE who take their bug reports very seriously and who are usually on top of their reports. A change that might seem trivial to you can still have serious unintended consequences. Martin isn't saying this out of malice or anything. He's just very careful in minor releases out of experience. To move this forward: Can you answer Martin's questions?
He didn't reply to Martin but to my speculations on why this patch wasn't dealt (I wasn't sure whether Martin is currently abroad or so and didn't want to leave him alone) Turns out that Martin isn't away and the bug probably just dropped off the table. I was unfortunately a bit straight in expressing that the issue is rather minor and esp. not really an html injection (what would be a severe bug but is not possible in this context at all) and am really sorry because I may have offended others expectations.
(In reply to comment #11) > He didn't reply to Martin but to my speculations on why this patch wasn't > dealt > (I wasn't sure whether Martin is currently abroad or so and didn't want to > leave him alone) > > Turns out that Martin isn't away and the bug probably just dropped off the > table. > > I was unfortunately a bit straight in expressing that the issue is rather > minor and esp. not really an html injection (what would be a severe bug but > is not possible in this context at all) and am really sorry because I may > have offended others expectations. Surely QLabel has a subset of HTML, sorry I should have said RichText: http://qt-project.org/doc/qt-4.8/richtext-html-subset.html Surely it supports also images and you can for example include a nice png that uses the _many_ vulnerabilities discovered in libpng and still _unpatched_ in Qt 4.8.1 (that ships libpng 1.5.4 still!!!). So for dynamically linked libpng this is not an issue, for statically linked it is. One can argue that if you runned the code this is already enough to make stuff happens, but is not true, there might be other ways to hang a client, it is not so rare as you think. Potentially one can write a simple web page with html tags in the title and then a simple JavaScript that hangs the whole browser. Modern browsers should have their own dialog to prevent this, but eh... you're relying on other's work now to save your ass, don't you? ;) Said this we can all agree on the fact this is not a very common and severe security issue, change the topic if makes you feel better, but answering like you did is far from what I expected. Just FYI: when a person sends me a patch for an issuet the first two words I say are "Thank you" whatever the patch is for and how is written. I call it respect.
On Monday 14 May 2012 19:22:59 you wrote: > Surely it supports also images and you can for example include a nice png > that uses the _many_ vulnerabilities discovered in libpng and still > _unpatched_ in Qt 4.8.1 (that ships libpng 1.5.4 still!!!). > So for dynamically linked libpng this is not an issue, for statically linked > it is. ldd /home/martin/kde/lib/kde4/libexec/kwin_killer_helper | grep png libpng12.so.0 => /lib/x86_64-linux-gnu/libpng12.so.0 As a matter of fact it just does not matter whether it is possible or not to inject anything in that dialog. There is no security issue with it. It is not possible to have privilege escalation as the dialog is running as a user. To become a vulnerability another application has to be vulnerable in the first place. If that is achieved there are easier ways to compromise the system than html inject into the killer dialog and use at least one more vulnerability there. > but answering like you did is far from what I expected. actually I think it's quite valid to figure out whether there is a real threat or if it is just a rendering issue. Given that a discussion of technical merits is quite important and of course we have to judge whether a fix should be considered for a bug fix release if we are internally days before the next feature release. > Just FYI: when a person sends me a patch for an issuet the first two words I > say are "Thank you" whatever the patch is for and how is written. I call it > respect. Let me say "Thank you" and sorry that I missed the patch. We have a high number of bug comments each day and it's very easy to miss a single comment. We don't use the bug tracker to exchange patches, which means it's not the place to look for those and to review those.
(In reply to comment #12) > Surely it supports also images > and you can for example include a nice png yes, local ones - you didn't actually try "kdialog --msgbox '<img src="http://some.img">'", did you? so what you have to do to cause somthing is: a) have a Qt client with vulnerable static libpng (no distro, i'd say) b) have a malicious png c) *store it in a known local path* d) alter the title of the browser e) hang the browser (you won't do that in a java script but we will assume you're doing this with some other client you control anyway) f) have the user click the close button (instead of killing the browser) to hopefully gain control over a kdialog process with user rights. I'd frankly stop when i can do (c) and do worse instead (like fixing crontabs or so) So, thank you for pointing out that we've a rendering issue in the kill helper dialog - could occasionally be nasty or even confusing - but unless you have an exploit for a compromising html injection (actually no matter whether it requires a weak browser, vulnerability chains are still a valid issue) that does not include a former security breach, it's not sth. severe, sorry.
Git commit a8cdd876878ffebb4a550613bc41a7fcbc8c3995 by Martin Gräßlin. Committed on 19/05/2012 at 10:20. Pushed by graesslin into branch 'master'. Escape HTML tags in window caption in Aurorae and Killer The unescaped tags are interpreted as HTML by Aurorae decorations and the KWin killer. Escaping the tags ensures that the text is not rendered incorrectly. FIXED-IN: 4.9.0 REVIEW: 104989 M +1 -0 kwin/clients/aurorae/src/qml/aurorae.qml M +5 -0 kwin/killer/killer.cpp http://commits.kde.org/kde-workspace/a8cdd876878ffebb4a550613bc41a7fcbc8c3995