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

Bug#769787: unblock (pre-approval): poco 1.3.6p1-5



On Mon, 17 Nov 2014 23:25:25 +0100
Niels Thykier <niels@thykier.net> wrote:

> Control: tags -1 moreinfo
> 
> On 2014-11-16 13:52, Cristian Greco wrote:
> > Package: release.debian.org
> > Severity: normal
> > User: release.debian.org@packages.debian.org
> > Usertags: unblock
> > 
> > Hi,
> > 
> > I'm seeking approval for the upload of poco 1.3.6p1-5.
> > 
> 
> Hi Cristian,
> 
> Thanks for doing this.
> 
> > I received a patch for CVE-2014-0350 (#746637) from Maxime Chatelle,
> > who prepared an upload for unstable.
> > 
> > Debdiff is attached, dsc is available on mentors:
> > http://mentors.debian.net/debian/pool/main/p/poco/poco_1.3.6p1-5.dsc
> > 
> > I don't see problems with the patch but would like approval from the
> > release team before going ahead. If we don't apply the patch now, poco
> > 1.3.6p1-4 should be removed from testing.
> > 
> > Thanks,
> > --
> > Cristian Greco
> > GPG key ID: 0xCF4D32E4
> > 
> > [...]
> > +diff -urNad poco-1.3.6p1~/NetSSL_OpenSSL/include/Poco/Net/X509Certificate.h poco-1.3.6p1/NetSSL_OpenSSL/include/Poco/Net/X509Certificate.h
> > +--- poco-1.3.6p1~/NetSSL_OpenSSL/include/Poco/Net/X509Certificate.h	2009-12-21 19:15:02.000000000 +0100
> > ++++ poco-1.3.6p1/NetSSL_OpenSSL/include/Poco/Net/X509Certificate.h	2014-11-07 22:09:56.519596616 +0100
> > +@@ -102,7 +102,7 @@
> > + 		
> > + protected:
> > + 	static bool containsWildcards(const std::string& commonName);
> > +-	static bool matchByAlias(const std::string& alias, const HostEntry& heData);
> > ++	static bool matchWildcard(const std::string& wildcard, const std::string& hostName);
> > + 	
> > [...]
> 
> 
> I suspect we might have an issue here.  Changing the name (and/or
> arguments) of a protected symbol is definitely an ABI breakage.  Since
> this is part of a public header it is also an API breakage.
> 
> Unfortunatly, I cannot accept the changes as-is.  That said, if you can
> provide an alternative solution without breaking neither ABI nor API, I
> would be interested in seeing it.

Hi Niels,

please find attached a new debdiff for review.

Maxime reworked the patch to avoid ABI/API breakages by adding a new
static function. The changes have been approved by upstream authors:

https://github.com/pocoproject/poco/issues/615

Thanks,
--
Cristian Greco
GPG key ID: 0xCF4D32E4
diff -u poco-1.3.6p1/debian/control poco-1.3.6p1/debian/control
--- poco-1.3.6p1/debian/control
+++ poco-1.3.6p1/debian/control
@@ -1,7 +1,7 @@
 Source: poco
 Priority: optional
 Maintainer: Cristian Greco <cristian@debian.org>
-Uploaders: Patrick Gansterer <paroga@paroga.com>
+Uploaders: Patrick Gansterer <paroga@paroga.com>, Maxime Chatelle <xakz@rxsoft.eu>
 Build-Depends: debhelper (>= 5), dpatch, libexpat1-dev, libmysqlclient-dev, libpcre3-dev (>= 7.8), libsqlite3-dev (>= 3.6.13), libssl-dev (>= 0.9.8), unixodbc-dev, zlib1g-dev
 Standards-Version: 3.8.3
 Section: libs
diff -u poco-1.3.6p1/debian/changelog poco-1.3.6p1/debian/changelog
--- poco-1.3.6p1/debian/changelog
+++ poco-1.3.6p1/debian/changelog
@@ -1,3 +1,11 @@
+poco (1.3.6p1-5) unstable; urgency=medium
+
+  * Adds debian/patches/70_fix_CVE-2014-0350.dpatch (Closes: #746637).
+    The patch is backported from poco-1.4.7 where the vulnerability
+    has been fixed.
+
+ -- Maxime Chatelle <xakz@rxsoft.eu>  Tue, 18 Nov 2014 11:45:20 +0100
+
 poco (1.3.6p1-4) unstable; urgency=low
 
   * Wheezy cleanup release (3): should fix FTBFS on GNU/kFreeBSD for real.
diff -u poco-1.3.6p1/debian/patches/00list poco-1.3.6p1/debian/patches/00list
--- poco-1.3.6p1/debian/patches/00list
+++ poco-1.3.6p1/debian/patches/00list
@@ -6,0 +7 @@
+70_fix_CVE-2014-0350.dpatch
only in patch2:
unchanged:
--- poco-1.3.6p1.orig/debian/patches/70_fix_CVE-2014-0350.dpatch
+++ poco-1.3.6p1/debian/patches/70_fix_CVE-2014-0350.dpatch
@@ -0,0 +1,115 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## 70_fix_CVE-2014-0350.dpatch by Maxime Chatelle <xakz@rxsoft.eu>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: Backported fix against CVE-2014-0350
+
+@DPATCH@
+
+--- poco-1.3.6p1-4/NetSSL_OpenSSL/src/X509Certificate.cpp	2009-12-21 19:15:02.000000000 +0100
++++ poco-1.3.6p1-5/NetSSL_OpenSSL/src/X509Certificate.cpp	2014-11-18 11:41:26.033979022 +0100
+@@ -48,6 +48,21 @@
+ #include <openssl/x509v3.h>
+ #include <openssl/err.h>
+ 
++static bool matchWildcard(const std::string& wildcard, const std::string& hostName)
++{
++	// fix wildcards
++	std::string wildcardExpr("^");
++	wildcardExpr += Poco::replace(wildcard, ".", "\\.");
++	Poco::replaceInPlace(wildcardExpr, "*", ".*");
++	Poco::replaceInPlace(wildcardExpr, "..*", ".*");
++	Poco::replaceInPlace(wildcardExpr, "?", ".?");
++	Poco::replaceInPlace(wildcardExpr, "..?", ".?");
++	wildcardExpr += "$";
++
++	Poco::RegularExpression expr(wildcardExpr, Poco::RegularExpression::RE_CASELESS);
++	return expr.match(hostName);
++}
++
+ 
+ namespace Poco {
+ namespace Net {
+@@ -107,51 +122,47 @@
+ 	std::string commonName;
+ 	std::set<std::string> dnsNames;
+ 	certificate.extractNames(commonName, dnsNames);
++	if (!commonName.empty()) dnsNames.insert(commonName);
+ 	bool ok = (dnsNames.find(hostName) != dnsNames.end());
+-
+-	char buffer[NAME_BUFFER_SIZE];
+-	X509_NAME* subj = 0;
+-	if (!ok && (subj = X509_get_subject_name(const_cast<X509*>(certificate.certificate()))) && X509_NAME_get_text_by_NID(subj, NID_commonName, buffer, sizeof(buffer)) > 0)
++	if (!ok)
+ 	{
+-		buffer[NAME_BUFFER_SIZE - 1] = 0;
+-		std::string commonName(buffer); // commonName can contain wildcards like *.appinf.com
+-		try
++		for (std::set<std::string>::const_iterator it = dnsNames.begin(); !ok && it != dnsNames.end(); ++it)
+ 		{
+-			// two cases: strData contains wildcards or not
+-			if (containsWildcards(commonName))
++			try
+ 			{
+-				// a compare by IPAddress is not possible with wildcards
+-				// only allow compare by name
+-				const HostEntry& heData = DNS::resolve(hostName);
+-				ok = matchByAlias(commonName, heData);
+-			}
+-			else
+-			{
+-				// it depends on hostName if we compare by IP or by alias
+-				IPAddress ip;
+-				if (IPAddress::tryParse(hostName, ip))
++				// two cases: strData contains wildcards or not
++				if (containsWildcards(*it))
+ 				{
+-					// compare by IP
+-					const HostEntry& heData = DNS::resolve(commonName);
+-					const HostEntry::AddressList& addr = heData.addresses();
+-					HostEntry::AddressList::const_iterator it = addr.begin();
+-					HostEntry::AddressList::const_iterator itEnd = addr.end();
+-					for (; it != itEnd && !ok; ++it)
+-					{
+-						ok = (*it == ip);
+-					}
++					// a compare by IPAddress is not possible with wildcards
++					// only allow compare by name
++					ok = matchWildcard(*it, hostName);
+ 				}
+ 				else
+ 				{
+-					// compare by name
+-					const HostEntry& heData = DNS::resolve(hostName);
+-					ok = matchByAlias(commonName, heData);
++					// it depends on hostName if we compare by IP or by alias
++					IPAddress ip;
++					if (IPAddress::tryParse(hostName, ip))
++					{
++						// compare by IP
++						const HostEntry& heData = DNS::resolve(*it);
++						const HostEntry::AddressList& addr = heData.addresses();
++						HostEntry::AddressList::const_iterator it = addr.begin();
++						HostEntry::AddressList::const_iterator itEnd = addr.end();
++						for (; it != itEnd && !ok; ++it)
++						{
++							ok = (*it == ip);
++						}
++					}
++					else
++					{
++						ok = Poco::icompare(*it, hostName) == 0;
++					}
+ 				}
+ 			}
+-		}
+-		catch (HostNotFoundException&)
+-		{
+-			return X509_V_ERR_APPLICATION_VERIFICATION;
++			catch (HostNotFoundException&)
++			{
++                		return X509_V_ERR_APPLICATION_VERIFICATION;
++			}
+ 		}
+ 	}
+ 

Attachment: pgpRmqx4DQrMc.pgp
Description: OpenPGP digital signature


Reply to: