CVE-2013-2074: KDE kdelibs Password Exposure

This was a low impact vulnerability reported by m.wege. The issue occurs on “internal server error” and triggers the below code located at kioslave/http/http.cpp.

/**
 * This function will read in the return header from the server.  It will
 * not read in the body of the return message.  It will also not transmit
 * the header to our client as the client doesn't need to know the gory
 * details of HTTP headers.
 */
bool HTTPProtocol::readResponseHeader()
{
   ...
    if (m_request.responseCode >= 500 && m_request.responseCode <= 599) {
        // Server side errors

        if (m_request.method == HTTP_HEAD) {
   ...
            if (!sendErrorPageNotification()) {
                error(ERR_INTERNAL_SERVER, m_request.url.url());
                return false;
   ...
    } else if (!isAuthenticationRequired(m_request.responseCode) && m_request.responseCode >= 400 && m_request.responseCode <= 499) {
        // Any other client errors
        // Tell that we will only get an error page here.
        if (!sendErrorPageNotification()) {
            if (m_request.responseCode == 403)
                error(ERR_ACCESS_DENIED, m_request.url.url());
            else
                error(ERR_DOES_NOT_EXIST, m_request.url.url());
            return false;
        }
   ...
}

Clearly, error() could be called passing the error number along with a string which in all of the above cases is the result of m_request.url.url().

void HTTPProtocol::error( int _err, const QString &_text )
{
  // Close the connection only on connection errors. Otherwise, honor the
  // keep alive flag.
  if (_err == ERR_CONNECTION_BROKEN || _err == ERR_COULD_NOT_CONNECT)
      httpClose(false);
  else
      httpClose(m_request.isKeepAlive);

  if (!m_request.id.isEmpty())
  {
    forwardHttpResponseHeader();
    sendMetaData();
  }

  // It's over, we don't need it anymore
  clearPostDataBuffer();

  SlaveBase::error( _err, _text );
  m_iError = _err;
}

The url() routine is part of kdecore/io/kurl.cpp file and it is being to used to dump the URL.

QString KUrl::url( AdjustPathOption trailing ) const
{
  if (QString::compare(scheme(), QLatin1String("mailto"), Qt::CaseInsensitive) == 0) {
      // mailto urls should be prettified, see the url183433 testcase.
      return prettyUrl(trailing);
  }
  if ( trailing == AddTrailingSlash && !path().endsWith( QLatin1Char('/') ) ) {
      // -1 and 0 are provided by QUrl, but not +1, so that one is a bit tricky.
      // To avoid reimplementing toEncoded() all over again, I just use another QUrl
      // Let's hope this is fast, or not called often...
      QUrl newUrl( *this );
      newUrl.setPath( path() + QLatin1Char('/') );
      return QString::fromLatin1(newUrl.toEncoded());
  }
  else if ( trailing == RemoveTrailingSlash) {
      const QString cleanedPath = trailingSlash(trailing, path());
      if (cleanedPath == QLatin1String("/")) {
          if (path() != QLatin1String("/")) {
              QUrl fixedUrl = *this;
              fixedUrl.setPath(cleanedPath);
              return QLatin1String(fixedUrl.toEncoded(None));
          }
          return QLatin1String(toEncoded(None));
      }
  }
  return QString::fromLatin1(toEncoded(trailing == RemoveTrailingSlash ? StripTrailingSlash : None));
}

Of course, this means that the same also applies when the URL includes username and password information such as http://username:password@example.com. To fix this the calls to url() were replaced by prettyUrl() with the following patch.

diff --git a/kioslave/http/http.cpp b/kioslave/http/http.cpp
index 2d139a9..129fc7b 100644
--- a/kioslave/http/http.cpp
+++ b/kioslave/http/http.cpp
@@ -3056,7 +3056,7 @@ try_again:
             ; // Ignore error
         } else {
             if (!sendErrorPageNotification()) {
-                error(ERR_INTERNAL_SERVER, m_request.url.url());
+                error(ERR_INTERNAL_SERVER, m_request.url.prettyUrl());
                 return false;
             }
         }
@@ -3072,9 +3072,9 @@ try_again:
         // Tell that we will only get an error page here.
         if (!sendErrorPageNotification()) {
             if (m_request.responseCode == 403)
-                error(ERR_ACCESS_DENIED, m_request.url.url());
+                error(ERR_ACCESS_DENIED, m_request.url.prettyUrl());
             else
-                error(ERR_DOES_NOT_EXIST, m_request.url.url());
+                error(ERR_DOES_NOT_EXIST, m_request.url.prettyUrl());
             return false;
         }
     } else if (m_request.responseCode >= 301 && m_request.responseCode<= 303) {

This routine among other things removes the password from the URL.

QString KUrl::prettyUrl( AdjustPathOption trailing ) const
{
  // reconstruct the URL in a "pretty" form
  // a "pretty" URL is NOT suitable for data transfer. It's only for showing data to the user.
  // however, it must be parseable back to its original state, since
  // notably Konqueror displays it in the Location address.

  // A pretty URL is the same as a normal URL, except that:
  // - the password is removed
  // - the hostname is shown in Unicode (as opposed to ACE/Punycode)
  // - the pathname and fragment parts are shown in Unicode (as opposed to %-encoding)
  QString result = scheme();
    ...
}