[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#626842: pu: package kde4libs/4:4.4.5-2+squeeze2



Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: pu

Hello,

[ Disclaimer: I've already asked security team about this upload and they told
me to do it via s-p-u ]

The upload would fix 3 CVEs and bug #612675. Change-by-change details are below
while full diff is attached.

* Fix CVE-2011-1168 (Konqueror partially universal XSS in error pages) by
  cve_2011_1168_konqueror_xss.diff.

  http://git.debian.org/?p=pkg-kde/kde-sc/kde4libs.git;a=commit;h=20deb674

* Fix CVE-2010-3170 (browser wildcard cerficate validation weakness) for
  Konqueror by cve_2010_3170_cn_wildcards.diff.

  http://git.debian.org/?p=pkg-kde/kde-sc/kde4libs.git;a=commit;h=ae934a0a

* Fix CVE-2011-1094 (kdelibs does not properly verify that the server hostname
  matches the Common Name of the Subject of an X.509 certificate if that CN is
  an IP address) by cve_2011_1094_ssl_verify_hostname.diff.

  http://git.debian.org/?p=pkg-kde/kde-sc/kde4libs.git;a=commit;h=2bfb1e47

  [ kde4libs non-security changes ]

* KTar: use unsigned arithmetic when calculating checksum of tar header record
  (as per ustar specification). However, when reading archive, verify
  checksum by calculating it both ways (unsigned and signed) and accept if
  either matches (partially solves #612675). Implemented in
  ktar_header_checksum_fix.diff patch.

  http://git.debian.org/?p=pkg-kde/kde-sc/kde4libs.git;a=commit;h=af9374ec

* Fix KTar longlink support when filenames are encoded in the UTF-8 (or other
  multibyte) locale. Implemented in ktar_longlink_length_in_bytes.diff patch
  (thanks to Ibragimov Rinat). Closes: #612675

  http://git.debian.org/?p=pkg-kde/kde-sc/kde4libs.git;a=commit;h=66efdda4

-- System Information:
Debian Release: wheezy/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing'), (110, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.38-2-amd64 (SMP w/4 CPU cores)
Locale: LANG=lt_LT.UTF-8, LC_CTYPE=lt_LT.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
diff --git a/debian/changelog b/debian/changelog
index 7e056e6..aac9418 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,26 @@
+kde4libs (4:4.4.5-2+squeeze2) UNRELEASED; urgency=low
+
+  [ José Manuel Santamaría Lema ]
+  * Fix CVE-2011-1168 (Konqueror partially universal XSS in error pages) by
+    cve_2011_1168_konqueror_xss.diff.
+  * Fix CVE-2010-3170 (browser wildcard cerficate validation weakness) for
+    Konqueror by cve_2010_3170_cn_wildcards.diff.
+  * Fix CVE-2011-1094 (kdelibs does not properly verify that the server hostname
+    matches the Common Name of the Subject of an X.509 certificate if that CN is
+    an IP address) by cve_2011_1094_ssl_verify_hostname.diff.
+
+  [ Modestas Vainius ]
+  * KTar: use unsigned arithmetic when calculating checksum of tar header record
+    (as per ustar specification). However, when reading archive, verify
+    checksum by calculating it both ways (unsigned and signed) and accept if
+    either matches (partially solves #612675). Implemented in
+    ktar_header_checksum_fix.diff patch.
+  * Fix KTar longlink support when filenames are encoded in the UTF-8 (or other
+    multibyte) locale. Implemented in ktar_longlink_length_in_bytes.diff patch
+    (thanks to Ibragimov Rinat). Closes: #612675
+
+ -- José Manuel Santamaría Lema <panfaust@gmail.com>  Tue, 12 Apr 2011 21:16:20 +0200
+
 kde4libs (4:4.4.5-2+squeeze1) stable-proposed-updates; urgency=low
 
   * Add a kconf_update script (migrate_from_kde3_icon_theme) to migrate away
diff --git a/debian/patches/cve_2010_3170_cn_wildcards.diff b/debian/patches/cve_2010_3170_cn_wildcards.diff
new file mode 100644
index 0000000..640252b
--- /dev/null
+++ b/debian/patches/cve_2010_3170_cn_wildcards.diff
@@ -0,0 +1,84 @@
+Origin: https://projects.kde.org/projects/kde/kdelibs/repository/revisions/f2a059e6
+Description: Fix wildcard ssl handling.
+ We now correctly handle wildcards, rather than using shell globs. This removes
+ the same issue as QTBUG-4455. In addition, fixes CVE-2010-3170 for Konqueror.
+ References:
+ * http://www.westpoint.ltd.uk/advisories/wp-10-0001.txt
+--- a/kio/kio/tcpslavebase.cpp
++++ b/kio/kio/tcpslavebase.cpp
+@@ -4,6 +4,7 @@
+  * Copyright (C) 2001 Dawit Alemayehu <adawit@kde.org>
+  * Copyright (C) 2007,2008 Andreas Hartmetz <ahartmetz@gmail.com>
+  * Copyright (C) 2008 Roland Harnau <tau@gmx.eu>
++ * Copyright (C) 2010 Richard Moore <rich@kde.org>
+  *
+  * This file is part of the KDE project
+  *
+@@ -436,6 +437,49 @@ bool TCPSlaveBase::startSsl()
+     return startTLSInternal(KTcpSocket::TlsV1) & ResultOk;
+ }
+ 
++// Find out if a hostname matches an SSL certificate's Common Name (including wildcards)
++static bool isMatchingHostname(const QString &cnIn, const QString &hostnameIn)
++{
++    const QString cn = cnIn.toLower();
++    const QString hostname = hostnameIn.toLower();
++
++    const int wildcard = cn.indexOf(QLatin1Char('*'));
++
++    // Check this is a wildcard cert, if not then just compare the strings
++    if (wildcard < 0)
++        return cn == hostname;
++
++    const int firstCnDot = cn.indexOf(QLatin1Char('.'));
++    const int secondCnDot = cn.indexOf(QLatin1Char('.'), firstCnDot+1);
++
++    // Check at least 3 components
++    if ((-1 == secondCnDot) || (secondCnDot+1 >= cn.length()))
++        return false;
++
++    // Check * is last character of 1st component (ie. there's a following .)
++    if (wildcard+1 != firstCnDot)
++        return false;
++
++    // Check only one star
++    if (cn.lastIndexOf(QLatin1Char('*')) != wildcard)
++        return false;
++
++    // Check characters preceding * (if any) match
++    if (wildcard && (hostname.leftRef(wildcard) != cn.leftRef(wildcard)))
++        return false;
++
++    // Check characters following first . match
++    if (hostname.midRef(hostname.indexOf(QLatin1Char('.'))) != cn.midRef(firstCnDot))
++        return false;
++
++    // Check if the hostname is an IP address, if so then wildcards are not allowed
++    QHostAddress addr(hostname);
++    if (!addr.isNull())
++        return false;
++
++    // Ok, I guess this was a wildcard CN and the hostname matches.
++    return true;
++}
+ 
+ TCPSlaveBase::SslResult TCPSlaveBase::startTLSInternal(uint v_)
+ {
+@@ -492,7 +536,6 @@ TCPSlaveBase::SslResult TCPSlaveBase::startTLSInternal(uint v_)
+     QSslCertificate peerCert = d->socket.peerCertificateChain().first();
+     QStringList domainPatterns(peerCert.subjectInfo(QSslCertificate::CommonName));
+     domainPatterns += peerCert.alternateSubjectNames().values(QSsl::DnsEntry);
+-    QRegExp domainMatcher(QString(), Qt::CaseInsensitive, QRegExp::Wildcard);
+     QMutableListIterator<KSslError> it(d->sslErrors);
+     while (it.hasNext()) {
+         // As of 4.4.0 Qt does not assign a certificate to the QSslError it emits
+@@ -503,8 +546,7 @@ TCPSlaveBase::SslResult TCPSlaveBase::startTLSInternal(uint v_)
+             continue;
+         }
+         foreach (const QString &dp, domainPatterns) {
+-            domainMatcher.setPattern(dp);
+-            if (domainMatcher.exactMatch(d->host)) {
++            if (isMatchingHostname(dp,d->host)) {
+                 it.remove();
+             }
+         }
diff --git a/debian/patches/cve_2011_1094_ssl_verify_hostname.diff b/debian/patches/cve_2011_1094_ssl_verify_hostname.diff
new file mode 100644
index 0000000..4971a3f
--- /dev/null
+++ b/debian/patches/cve_2011_1094_ssl_verify_hostname.diff
@@ -0,0 +1,51 @@
+Origin: https://projects.kde.org/projects/kde/kdelibs/repository/revisions/3735e2ee
+Description: Harden SSL verification against poisoned DNS attacks
+ ... in the case of certificates that are issued against an IP address rather
+ than a hostname.
+ Patch by Tomas Hoger / Red Hat Security Response Team, reviewed by Jeff
+ Mitchell and Richard Moore.
+--- a/kio/kio/tcpslavebase.cpp
++++ b/kio/kio/tcpslavebase.cpp
+@@ -534,23 +534,34 @@ TCPSlaveBase::SslResult TCPSlaveBase::startTLSInternal(uint v_)
+     // domain<->certificate matching here.
+     d->sslErrors = d->socket.sslErrors();
+     QSslCertificate peerCert = d->socket.peerCertificateChain().first();
+-    QStringList domainPatterns(peerCert.subjectInfo(QSslCertificate::CommonName));
+-    domainPatterns += peerCert.alternateSubjectNames().values(QSsl::DnsEntry);
+     QMutableListIterator<KSslError> it(d->sslErrors);
+     while (it.hasNext()) {
+         // As of 4.4.0 Qt does not assign a certificate to the QSslError it emits
+         // *in the case of HostNameMismatch*. A HostNameMismatch, however, will always
+         // be an error of the peer certificate so we just don't check the error's
+         // certificate().
+-        if (it.next().error() != KSslError::HostNameMismatch) {
+-            continue;
++
++        // Remove all HostNameMismatch, we have to redo name checking later.
++        if (it.next().error() == KSslError::HostNameMismatch) {
++			it.remove();
+         }
+-        foreach (const QString &dp, domainPatterns) {
+-            if (isMatchingHostname(dp,d->host)) {
+-                it.remove();
+-            }
++    }
++    // Redo name checking here and (re-)insert HostNameMismatch to sslErrors if
++    // host name does not match any of the names in server certificate.
++    // QSslSocket may not report HostNameMismatch error, when server
++    // certificate was issued for the IP we are connecting to.
++    QStringList domainPatterns(peerCert.subjectInfo(QSslCertificate::CommonName));
++    domainPatterns += peerCert.alternateSubjectNames().values(QSsl::DnsEntry);
++    bool names_match = false;
++    foreach (const QString &dp, domainPatterns) {
++        if (isMatchingHostname(dp,d->host)) {
++            names_match = true;
++            break;
+         }
+     }
++    if (!names_match) {
++        d->sslErrors.insert(0, KSslError(KSslError::HostNameMismatch, peerCert));
++    }
+ 
+     // The app side needs the metadata now for the SSL error dialog (if any) but
+     // the same metadata will be needed later, too. When "later" arrives the slave
diff --git a/debian/patches/cve_2011_1168_konqueror_xss.diff b/debian/patches/cve_2011_1168_konqueror_xss.diff
new file mode 100644
index 0000000..7725eee
--- /dev/null
+++ b/debian/patches/cve_2011_1168_konqueror_xss.diff
@@ -0,0 +1,19 @@
+Origin: https://projects.kde.org/projects/kde/kdelibs/repository/revisions/8b06e2c
+Description: This patch fixes CVE-2011-1168.
+ References:
+ * http://www.kde.org/info/security/advisory-20110411-1.txt
+ * http://www.nth-dimension.org.uk/pub/NDSA20110321.txt.asc
+--- a/khtml/khtml_part.cpp
++++ b/khtml/khtml_part.cpp
+@@ -1848,7 +1848,10 @@
+   stream >> errorName >> techName >> description >> causes >> solutions;
+ 
+   QString url, protocol, datetime;
+-  url = Qt::escape( reqUrl.prettyUrl() );
++
++  // This is somewhat confusing, but we have to escape the externally-
++  // controlled URL twice: once for i18n, and once for HTML.
++  url = Qt::escape( Qt::escape( reqUrl.prettyUrl() ) );
+   protocol = reqUrl.protocol();
+   datetime = KGlobal::locale()->formatDateTime( QDateTime::currentDateTime(),
+                                                 KLocale::LongDate );
diff --git a/debian/patches/ktar_header_checksum_fix.diff b/debian/patches/ktar_header_checksum_fix.diff
new file mode 100644
index 0000000..297aaac
--- /dev/null
+++ b/debian/patches/ktar_header_checksum_fix.diff
@@ -0,0 +1,94 @@
+From: Modestas Vainius <modax@debian.org>
+Subject: Use unsigned arithmetic when calculating tar header checksum
+Forwarded: yes
+Bug: https://bugs.kde.org/show_bug.cgi?id=266141
+Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=612675
+Last-Update: 2011-05-14
+Origin: vendor
+
+According to the ustar specification, implementations must use unsigned
+arithmetic when calculating checksum field of the tar header record. KTar prior
+to this patch used signed arithmetic for checksum calculation when writing an
+archive. The patch fixes this.
+
+However, there are more broken tar implementations out there (including former
+KTar itself) so the patch also makes KTar to verify checksums using both
+unsigned and signed arithmetic when reading archives. If either of checksums
+matches, archive is accepted.
+
+--- a/kio/kio/ktar.cpp
++++ b/kio/kio/ktar.cpp
+@@ -198,26 +198,41 @@ qint64 KTar::KTarPrivate::readRawHeader(
+     if (strncmp(buffer + 257, "ustar", 5)) {
+       // The magic isn't there (broken/old tars), but maybe a correct checksum?
+ 
+-      int check = 0;
+-      for( uint j = 0; j < 0x200; ++j )
+-        check += buffer[j];
++      // Checksum is supposed to be a sum of unsigned bytes but some
++      // implementations sum signed chars. Therefore, just check both.
++      int check_unsigned = 0, check_signed = 0;
++      for( uint j = 0; j < 0x200; ++j ) {
++        check_unsigned += (unsigned char) buffer[j];
++        check_signed += buffer[j];
++      }
+ 
+       // adjust checksum to count the checksum fields as blanks
+-      for( uint j = 0; j < 8 /*size of the checksum field including the \0 and the space*/; j++ )
+-        check -= buffer[148 + j];
+-      check += 8 * ' ';
+-
+-      QByteArray s = QByteArray::number( check, 8 ); // octal
+-
+-      // only compare those of the 6 checksum digits that mean something,
+-      // because the other digits are filled with all sorts of different chars by different tars ...
+-      // Some tars right-justify the checksum so it could start in one of three places - we have to check each.
+-      if( strncmp( buffer + 148 + 6 - s.length(), s.data(), s.length() )
+-        && strncmp( buffer + 148 + 7 - s.length(), s.data(), s.length() )
+-        && strncmp( buffer + 148 + 8 - s.length(), s.data(), s.length() ) ) {
++      for( uint j = 0; j < 8 /*size of the checksum field including the \0 and the space*/; j++ ) {
++        check_unsigned -= (unsigned char) buffer[148 + j];
++        check_signed -= buffer[148 + j];
++      }
++      check_unsigned += 8 * ' ';
++      check_signed += 8 * ' ';
++
++      QList<QByteArray> checks;
++      checks << QByteArray::number( check_unsigned, 8 ); // octal
++      if (check_unsigned != check_signed)
++        checks << QByteArray::number( check_signed, 8 ); // octal
++
++      bool checksum_ok = false;
++      foreach (QByteArray s, checks) {
++        // only compare those of the 6 checksum digits that mean something,
++        // because the other digits are filled with all sorts of different chars by different tars ...
++        // Some tars right-justify the checksum so it could start in one of three places - we have to check each.
++        if ((checksum_ok = !strncmp( buffer + 148 + 6 - s.length(), s.data(), s.length() )
++          || !strncmp( buffer + 148 + 7 - s.length(), s.data(), s.length() )
++          || !strncmp( buffer + 148 + 8 - s.length(), s.data(), s.length() )))
++            break;
++      }
++      if (!checksum_ok) {
+         kWarning(7041) << "KTar: invalid TAR file. Header is:" << QByteArray( buffer+257, 5 )
+                        << "instead of ustar. Reading from wrong pos in file?"
+-                       << "checksum=" << QByteArray( buffer + 148 + 6 - s.length(), s.length() );
++                       << "checksum=" << QByteArray( buffer + 148, 6 );
+         return -1;
+       }
+     }/*end if*/
+@@ -658,10 +673,12 @@ void KTar::KTarPrivate::fillBuffer( char
+   // group
+   strcpy( buffer + 0x129, gname );
+ 
+-  // Header check sum
+-  int check = 32;
++  // For checksumming purposes, all checksum[8] bytes must be 0x20. The 7th
++  // checksum byte (0x9a) byte is initialized to 0 above hence initialize
++  // 'check' value to 0x20 in order to compensate for that.
++  int check = 0x20;
+   for( uint j = 0; j < 0x200; ++j )
+-    check += buffer[j];
++    check += (unsigned char) buffer[j]; // checksum must use unsigned arithmetic
+   s = QByteArray::number( check, 8 ); // octal
+   s = s.rightJustified( 6, '0' );
+   memcpy( buffer + 0x94, s.constData(), 6 );
diff --git a/debian/patches/ktar_longlink_length_in_bytes.diff b/debian/patches/ktar_longlink_length_in_bytes.diff
new file mode 100644
index 0000000..8fc6d0b
--- /dev/null
+++ b/debian/patches/ktar_longlink_length_in_bytes.diff
@@ -0,0 +1,50 @@
+From: Ibragimov Rinat <ibragimovrinat@mail.ru>
+Subject: Fix longlink UTF-8 support in KTar
+Bug: http://bugs.kde.org/show_bug.cgi?id=266141
+Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=612675
+Last-Update: 2011-05-14
+Forwarded: yes
+Origin: other, http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=612675#5
+Acked-By: Modestas Vainius <modax@debian.org>
+
+tar archives have to use "longlink trick" to store names longer than 100 bytes.
+KTar class has functions implementing longlink, but they check name length in
+_characters_, not in bytes. For non-ASCII characters in UTF-8 length of string
+in bytes and length in characters do not match. In my case file had
+character-length less than 100 and byte-length greater than 100, so name simply
+truncated. Such behavior can be observed on non-ASCII UTF-8 or any other
+multibyte encoding. If file name is very long, resulting .tar may become
+unreadable.
+
+--- a/kio/kio/ktar.cpp
++++ b/kio/kio/ktar.cpp
+@@ -750,7 +750,7 @@ bool KTar::doPrepareWriting(const QStrin
+     const QByteArray gname = group.toLocal8Bit();
+ 
+     // If more than 100 chars, we need to use the LongLink trick
+-    if ( fileName.length() > 99 )
++    if ( encodedFileName.length() > 99 )
+         d->writeLonglink(buffer,encodedFileName,'L',uname,gname);
+ 
+     // Write (potentially truncated) name
+@@ -803,7 +803,7 @@ bool KTar::doWriteDir(const QString &nam
+     QByteArray gname = group.toLocal8Bit();
+ 
+     // If more than 100 chars, we need to use the LongLink trick
+-    if ( dirName.length() > 99 )
++    if ( encodedDirname.length() > 99 )
+         d->writeLonglink(buffer,encodedDirname,'L',uname,gname);
+ 
+     // Write (potentially truncated) name
+@@ -855,9 +855,9 @@ bool KTar::doWriteSymLink(const QString
+     QByteArray gname = group.toLocal8Bit();
+ 
+     // If more than 100 chars, we need to use the LongLink trick
+-    if (target.length() > 99)
++    if (encodedTarget.length() > 99)
+         d->writeLonglink(buffer,encodedTarget,'K',uname,gname);
+-    if ( fileName.length() > 99 )
++    if ( encodedFileName.length() > 99 )
+         d->writeLonglink(buffer,encodedFileName,'L',uname,gname);
+ 
+     // Write (potentially truncated) name
diff --git a/debian/patches/series b/debian/patches/series
index 44ca285..487a3b0 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -18,3 +18,8 @@
 29_hurd_support.diff
 30_kfileshare_kdesu_fileshareset.diff
 31_relax_plugin_kde_version_check.diff
+cve_2011_1168_konqueror_xss.diff
+cve_2010_3170_cn_wildcards.diff
+cve_2011_1094_ssl_verify_hostname.diff
+ktar_header_checksum_fix.diff
+ktar_longlink_length_in_bytes.diff

Reply to: