Security advisory from KDE upstream

[Rohan Garg]

Noticifations about errors sometimes contain password when using KDE.

There is a patch upstream over here https://projects.kde.org/projects/kde/kdelibs/repository/revisions/65d736dab592bced4410ccfa4699de89f78c96ca

All supported versions of kde4libs need to be patched

[M-wege (m-wege)]

I just received a notification from the ressource which read "internal server error" and the url https://username:<email address hidden>/remote.php.carddav...

I believe it is not a good idea to have a password in a notication.

Reproducible: Always

[Allen Winter (awinterz-deactivatedaccount)]

somewhere a message is using url() rather than prettyUrl().

but so far I haven't had any luck finding where in the code.

maybe another set of eyes will have more luck. should be an easy fix once we find the offending text.

[Montel-3 (montel-3)]

We need a screenshot or exact error message to find it.

[M-wege (m-wege)]

Is there a way to provoke a connection error? It doesn't work when just disconnecting the internet. The cause must have been on the server side, so I will only see the message again, when I can fake a server error or it happens again.

[Greg-xrvasas (greg-xrvasas)]

I think this was introduced by 649a97d08771020a4e5151bbc041e82405f5841c, at least that the only commit I can thin of that touched the error messages. If true, there are some chances that the issue comes from KIO.

[Greg-xrvasas (greg-xrvasas)]

Looks like the source is in kdelibs/kioslave/http/http.cpp:3059, where url() is used instead of prettyUrl() as the error message.

Do you think this can go into kdelibs for 4.10.4?

[Allen Winter (awinterz-deactivatedaccount)]

yes please.

looks like changing that line to use m_request.url.host() might be the correct solution.

In fact, once this is fixed I'll send a note to the packages that they might want to hotpatch their 4.10.3 releases.

[Greg-xrvasas (greg-xrvasas)]

(In reply to comment #6)
> looks like changing that line to use m_request.url.host() might be the
> correct solution.

Having the full URL that triggered this error would help finding the issue, so I'm not certain that just keeping the hostname would be satisfying to most users. As for the usage of this string there's a new line between 'Internal error in server' and the error text, which makes mes doubt that %1 stands for the hostname in the full message.

Otherwise, looking around this line the m_request.url.url() is also used in the same way (lines 3075 and 3077). I'll also replace those with prettyUrl().

[Greg-xrvasas (greg-xrvasas)]

Git commit 65d736dab592bced4410ccfa4699de89f78c96ca by Grégory Oestreicher.
Committed on 08/05/2013 at 23:16.
Pushed by goestreicher into branch 'KDE/4.10'.

Don't show passwords contained in HTTP URLs in error messages

M +3 -3 kioslave/http/http.cpp

http://commits.kde.org/kdelibs/65d736dab592bced4410ccfa4699de89f78c96ca

[Rohan Garg (rohangarg)]

Patch for oneiric

[Rohan Garg (rohangarg)]

Diff for precise

[Rohan Garg (rohangarg)]

Patch for Oneiric

[Rohan Garg (rohangarg)]

Patch for oneiric

[Rohan Garg (rohangarg)]

Patch for precise

[Rohan Garg (rohangarg)]

Patch for Quantal

[Rohan Garg (rohangarg)]

Patch for raring

[Rohan Garg (rohangarg)]

Maybe the changelog needs to be edited a bit before uploading, but the gist of the issue is that the user's password is shown in the notification

[Rohan Garg (rohangarg)]

This has been reported as CVE-2013-2074

[Kevin-kofler (kevin-kofler)]

While working on a fix for the Fedora kdelibs3 compatibility package, I noticed that your fix for 4.10 is NOT complete: There are at least 2 instances where url() (rather than prettyUrl()) is still used in error messages!

https://projects.kde.org/projects/kde/kdelibs/repository/entry/kioslave/http/http.cpp?rev=KDE%2F4.10#L1582
This one looks particularly weird: Only if the URL is NOT null, it gets replaced with the default??? I think the ! there is too much. But the main issue is that it uses url() and (later in the function) prints the thing.

https://projects.kde.org/projects/kde/kdelibs/repository/entry/kioslave/http/http.cpp?rev=KDE%2F4.10#L3467
And this one shouldn't need any explanation of why it's bad.

[Kevin-kofler (kevin-kofler)]

Created attachment 79887
kdelibs-3.5.10-CVE-2013-2074.patch

For reference (and for other distros which still ship kdelibs3), here's my tentative kdelibs3 patch. In addition to what you fixed, this patch also fixes the debugging output (fixed in 4.x by an earlier commit), the 2 spots I pointed out above, and 1 additional one I can't find in the 4.10 code (the error(ERR_ACCESS_DENIED, u.url()); line – it apparently got rewritten or removed).

[Greg-xrvasas (greg-xrvasas)]

(In reply to comment #9)
> While working on a fix for the Fedora kdelibs3 compatibility package, I
> noticed that your fix for 4.10 is NOT complete: There are at least 2
> instances where url() (rather than prettyUrl()) is still used in error
> messages!

Wow, thanks a lot for that.

> https://projects.kde.org/projects/kde/kdelibs/repository/entry/kioslave/http/
> http.cpp?rev=KDE%2F4.10#L1582
> This one looks particularly weird: Only if the URL is NOT null, it gets
> replaced with the default???

I think that url can be null is _url cannot be converted, so it looks legitimate. However you're right, prettyUrl() should indeed be used.

> https://projects.kde.org/projects/kde/kdelibs/repository/entry/kioslave/http/
> http.cpp?rev=KDE%2F4.10#L3467
> And this one shouldn't need any explanation of why it's bad.

Indeed.

Thanks a lot for you help, I'll fix those.

Cheers,
Grégory

[Greg-xrvasas (greg-xrvasas)]

Git commit 898135a59d91184692ed1bcee8bb4c6d80d6f7b9 by Grégory Oestreicher.
Committed on 15/05/2013 at 21:56.
Pushed by goestreicher into branch 'KDE/4.10'.

Continue hiding passwords in URLs displayed to the user
The fix introduced by 65d736da missed two usages of
url() instead of prettyUrl(). Thanks to Kevin Kofler
for spotting those.

M +2 -2 kioslave/http/http.cpp

http://commits.kde.org/kdelibs/898135a59d91184692ed1bcee8bb4c6d80d6f7b9

[Kevin-kofler (kevin-kofler)]

> I think that url can be null is _url cannot be converted, so it looks legitimate.

1. There's no conversion there. It's just a copy from a const QString & to a writable QString.
2. The thing there is that it checks for if (!url.isNull()) when I think it wants to actually check for if (url.isNull()) without the '!' (or maybe actually for if (url.isEmpty()), again without negation; checking strings for isNull is not recommended and rarely needed). The code looks to me like the intention was to fill in a default value if no URL was given, whereas as written now, it leaves it empty if it was empty and it overwrites it with the default value if it was not empty.

[Rohan Garg (rohangarg)]

Updated patch for raring

[Rohan Garg (rohangarg)]

Updated patch for Quantal

[Rohan Garg (rohangarg)]

Updated patch for Precise

[Jamie Strandboge (jdstrand)]

Thanks for the debdiffs! They are mostly fine, bug I have a couple of comments:
 * they only mention one of the commits in the patch headers. The code itself has both
    898135a59d91184692ed1bcee8bb4c6d80d6f7b9 and 65d736dab592bced4410ccfa4699de89f78c96ca, but the patch
    headers only list 65d736dab592bced4410ccfa4699de89f78c96ca.
 * the precise debdiff needed to have the patch refreshed
 * the raring debdiff does not properly apply because there is no trailing newline
 * while not required, typically the patch will include the CVE number. Ie, instead of kubuntu_use_pretty_url.diff you might use CVE-2013-2074.diff
 * the changelog does not use the format as described in https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation#Packaging

Eg, properly formatted changelog entry for -security might be:
kde4libs (4:4.9.5-0ubuntu0.2) quantal-security; urgency=low

  * SECURITY UPDATE: information disclosure via error notifications
    - debian/patches/kubuntu_use_pretty_url.diff: update
      kioslave/http/http.cpp to use prettyUrl()
    - CVE-2013-2074
    - LP: #1178286

I've gone ahead and fixed these issues and uploaded. Thanks again!

[Rohan Garg (rohangarg)]

Thanks alot Jamie :)

[Launchpad Janitor (janitor)]

This bug was fixed in the package kde4libs - 4:4.8.5-0ubuntu0.2

---------------
kde4libs (4:4.8.5-0ubuntu0.2) precise-security; urgency=low

  * SECURITY UPDATE: information disclosure via error notifications
    - debian/patches/kubuntu_use_pretty_url.diff: update
      kioslave/http/http.cpp to use prettyUrl()
    - CVE-2013-2074
    - LP: #1178286
 -- Rohan Garg <email address hidden> Thu, 09 May 2013 16:36:38 +0100

[Launchpad Janitor (janitor)]

This bug was fixed in the package kde4libs - 4:4.9.5-0ubuntu0.2

---------------
kde4libs (4:4.9.5-0ubuntu0.2) quantal-security; urgency=low

  * SECURITY UPDATE: information disclosure via error notifications
    - debian/patches/kubuntu_use_pretty_url.diff: update
      kioslave/http/http.cpp to use prettyUrl()
    - CVE-2013-2074
    - LP: #1178286
 -- Rohan Garg <email address hidden> Thu, 09 May 2013 17:04:18 +0100

[Launchpad Janitor (janitor)]

This bug was fixed in the package kde4libs - 4:4.10.2-0ubuntu2.2

---------------
kde4libs (4:4.10.2-0ubuntu2.2) raring-security; urgency=low

  * SECURITY UPDATE: information disclosure via error notifications
    - debian/patches/kubuntu_use_pretty_url.diff: update
      kioslave/http/http.cpp to use prettyUrl()
    - CVE-2013-2074
    - LP: #1178286
 -- Jamie Strandboge <email address hidden> Tue, 28 May 2013 16:12:34 -0500