From abf23ee86d871850347cfec80636537e9eb20ea6 Mon Sep 17 00:00:00 2001
From: Magnus Hagander <magnus@hagander.net>
Date: Wed, 9 Dec 2009 06:37:06 +0000
Subject: [PATCH] Reject certificates with embedded NULLs in the commonName
 field. This stops attacks where an attacker would put <attack>\0<propername>
 in the field and trick the validation code that the certificate was for
 <attack>.

This is a very low risk attack since it reuqires the attacker to trick the
CA into issuing a certificate with an incorrect field, and the common
PostgreSQL deployments are with private CAs, and not external ones. Also,
default mode in 8.4 does not do any name validation, and is thus also not
vulnerable - but the higher security modes are.

Backpatch all the way. Even though versions 8.3.x and before didn't have
certificate name validation support, they still exposed this field for
the user to perform the validation in the application code, and there
is no way to detect this problem through that API.

Security: CVE-2009-4034
---
 src/backend/libpq/be-secure.c    | 24 ++++++++++++++++++++++--
 src/interfaces/libpq/fe-secure.c | 25 ++++++++++++++++++++++---
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index ef2db7f1745..3e4756f1c7c 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.92 2009/06/11 14:48:58 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.93 2009/12/09 06:37:06 mha Exp $
  *
  *	  Since the server static private key ($DataDir/server.key)
  *	  will normally be stored unencrypted so that the database
@@ -953,9 +953,29 @@ aloop:
 		X509_NAME_oneline(X509_get_subject_name(port->peer),
 						  port->peer_dn, sizeof(port->peer_dn));
 		port->peer_dn[sizeof(port->peer_dn) - 1] = '\0';
-		X509_NAME_get_text_by_NID(X509_get_subject_name(port->peer),
+		r = X509_NAME_get_text_by_NID(X509_get_subject_name(port->peer),
 					   NID_commonName, port->peer_cn, sizeof(port->peer_cn));
 		port->peer_cn[sizeof(port->peer_cn) - 1] = '\0';
+		if (r == -1)
+		{
+			/* Unable to get the CN, set it to blank so it can't be used */
+			port->peer_cn[0] = '\0';
+		}
+		else
+		{
+			/*
+			 * Reject embedded NULLs in certificate common name to prevent attacks like
+			 * CVE-2009-4034.
+			 */
+			if (r != strlen(port->peer_cn))
+			{
+				ereport(COMMERROR,
+						(errcode(ERRCODE_PROTOCOL_VIOLATION),
+						 errmsg("SSL certificate's common name contains embedded null")));
+				close_SSL(port);
+				return -1;
+			}
+		}
 	}
 	ereport(DEBUG2,
 			(errmsg("SSL connection from \"%s\"", port->peer_cn)));
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index dbd37d50390..6f6052b2b05 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.128 2009/07/24 17:58:31 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.129 2009/12/09 06:37:06 mha Exp $
  *
  * NOTES
  *
@@ -1265,9 +1265,28 @@ open_client_SSL(PGconn *conn)
 					  conn->peer_dn, sizeof(conn->peer_dn));
 	conn->peer_dn[sizeof(conn->peer_dn) - 1] = '\0';
 
-	X509_NAME_get_text_by_NID(X509_get_subject_name(conn->peer),
+	r = X509_NAME_get_text_by_NID(X509_get_subject_name(conn->peer),
 							  NID_commonName, conn->peer_cn, SM_USER);
-	conn->peer_cn[SM_USER] = '\0';
+	conn->peer_cn[SM_USER] = '\0'; /* buffer is SM_USER+1 chars! */
+	if (r == -1)
+	{
+		/* Unable to get the CN, set it to blank so it can't be used */
+		conn->peer_cn[0] = '\0';
+	}
+	else
+	{
+		/*
+		 * Reject embedded NULLs in certificate common name to prevent attacks like
+		 * CVE-2009-4034.
+		 */
+		if (r != strlen(conn->peer_cn))
+		{
+			printfPQExpBuffer(&conn->errorMessage,
+							  libpq_gettext("SSL certificate's common name contains embedded null\n"));
+			close_SSL(conn);
+			return PGRES_POLLING_FAILED;
+		}
+	}
 
 	if (!verify_peer_name_matches_certificate(conn))
 	{
-- 
GitLab