Security advisory from KDE upstream

Bug #1178286 reported by Rohan Garg on 2013-05-09
258
This bug affects 1 person
Affects Status Importance Assigned to Milestone
kdelibs
Fix Released
Medium
kde4libs (Ubuntu)
Undecided
Unassigned
Declined for Lucid by Jonathan Riddell
Precise
Undecided
Unassigned
Quantal
Undecided
Unassigned
Raring
Undecided
Unassigned
Saucy
Undecided
Unassigned

Bug Description

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

CVE References

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

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.

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

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.

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.

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?

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.

(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().

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) on 2013-05-09
Changed in kde4libs (Ubuntu Saucy):
status: New → Fix Released
Rohan Garg (rohangarg) on 2013-05-09
tags: added: kubuntu
Changed in kdelibs:
importance: Unknown → Medium
status: Unknown → Fix Released
Rohan Garg (rohangarg) wrote :

Patch for oneiric

Rohan Garg (rohangarg) wrote :

Diff for precise

Rohan Garg (rohangarg) wrote :

Patch for Oneiric

Rohan Garg (rohangarg) wrote :

Patch for oneiric

Rohan Garg (rohangarg) wrote :

Patch for precise

Rohan Garg (rohangarg) wrote :

Patch for Quantal

Rohan Garg (rohangarg) wrote :

Patch for raring

Rohan Garg (rohangarg) wrote :

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) wrote :

This has been reported as CVE-2013-2074

information type: Public → Private Security
information type: Private Security → Public Security

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.

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).

(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

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

> 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) wrote :

Updated patch for raring

Rohan Garg (rohangarg) wrote :

Updated patch for Quantal

Rohan Garg (rohangarg) wrote :

Updated patch for Precise

Changed in kde4libs (Ubuntu Precise):
status: New → Confirmed
Changed in kde4libs (Ubuntu Quantal):
status: New → Confirmed
Changed in kde4libs (Ubuntu Raring):
status: New → Confirmed
Jamie Strandboge (jdstrand) wrote :

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!

Changed in kde4libs (Ubuntu Precise):
status: Confirmed → Fix Committed
Changed in kde4libs (Ubuntu Quantal):
status: Confirmed → Fix Committed
Changed in kde4libs (Ubuntu Raring):
status: Confirmed → Fix Committed
Rohan Garg (rohangarg) wrote :

Thanks alot Jamie :)

Launchpad Janitor (janitor) wrote :

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

Changed in kde4libs (Ubuntu Precise):
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :

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

Changed in kde4libs (Ubuntu Quantal):
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :

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

Changed in kde4libs (Ubuntu Raring):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.