[Pkg-postgresql-public] Bug#718460: libpq5 not thread-safe

[Nick Phillips]

Package: libpq5
Version: 9.1.9-1
Severity: normal
Tags: patch upstream

I've had crashes with glibc detecting double free/memory corruption through
mod_wsgi, Django, psycopg2, libpq, libssl.

Turns out that libpq is not thread-safe; it uses a global SSL_context and
does not protect its use of it appropriately, resulting in crashes in
libssl.

The point at which it had been affecting me was during the call to
SSL_CTX_use_certificate_chain_file in initialize_SSL in fe-secure.c

There are several further vulnerable uses of SSL_context later within that
function; the patch I'm attaching protects all of them using a single
call to pthread_mutex_lock -- I've not bothered to investigate whether
the subsequent uses of SSL_context are such that they require the context
not be modified by other threads in the interim, or whether it would be
safe to protect each use separately. I don't think the gain from doing
so would be significant anyway, as this section of code is only used once
during each connection setup.

While I am using postgres 9.1 from wheezy, I note that this issue is not
fixed in current postgres 9.3 beta2 upstream.


Cheers,


Nick

-- System Information:
Debian Release: 7.1
  APT prefers stable
  APT policy: (990, 'stable'), (500, 'stable-updates')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.32-5-xen-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_NZ.UTF-8, LC_CTYPE=en_NZ.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages libpq5 depends on:
ii  libc6             2.13-38
ii  libcomerr2        1.42.5-1.1
ii  libgssapi-krb5-2  1.10.1+dfsg-5+deb7u1
ii  libkrb5-3         1.10.1+dfsg-5+deb7u1
ii  libldap-2.4-2     2.4.31-1+nmu2
ii  libssl1.0.0       1.0.1e-2

libpq5 recommends no packages.

libpq5 suggests no packages.

-- no debconf information

===> patch follows

Description: libpq thread safety fix
 Protect libpq use of global SSL_context with mutex.
Author: Nick Phillips <nwp at debian.org>
Forwarded: no
Last-Update: 2013-08-01

--- postgresql-9.1-9.1.9.orig/src/interfaces/libpq/fe-secure.c
+++ postgresql-9.1-9.1.9/src/interfaces/libpq/fe-secure.c
@@ -1089,6 +1089,10 @@ initialize_SSL(PGconn *conn)
 		 * sslcert settings are used for different connections in the same
 		 * process.
 		 */
+#ifdef ENABLE_THREAD_SAFETY
+		if (pthread_mutex_lock(&ssl_config_mutex))
+			return -1;
+#endif
 		if (SSL_CTX_use_certificate_chain_file(SSL_context, fnbuf) != 1)
 		{
 			char	   *err = SSLerrmessage();
@@ -1097,6 +1101,9 @@ initialize_SSL(PGconn *conn)
 			   libpq_gettext("could not read certificate file \"%s\": %s\n"),
 							  fnbuf, err);
 			SSLerrfree(err);
+#ifdef ENABLE_THREAD_SAFETY
+			pthread_mutex_unlock(&ssl_config_mutex);
+#endif
 			return -1;
 		}
 		if (SSL_use_certificate_file(conn->ssl, fnbuf, SSL_FILETYPE_PEM) != 1)
@@ -1107,6 +1114,9 @@ initialize_SSL(PGconn *conn)
 			   libpq_gettext("could not read certificate file \"%s\": %s\n"),
 							  fnbuf, err);
 			SSLerrfree(err);
+#ifdef ENABLE_THREAD_SAFETY
+			pthread_mutex_unlock(&ssl_config_mutex);
+#endif
 			return -1;
 		}
 		/* need to load the associated private key, too */
@@ -1145,6 +1155,9 @@ initialize_SSL(PGconn *conn)
 								  engine_str, err);
 				SSLerrfree(err);
 				free(engine_str);
+#ifdef ENABLE_THREAD_SAFETY
+				pthread_mutex_unlock(&ssl_config_mutex);
+#endif
 				return -1;
 			}
 
@@ -1159,6 +1172,9 @@ initialize_SSL(PGconn *conn)
 				ENGINE_free(conn->engine);
 				conn->engine = NULL;
 				free(engine_str);
+#ifdef ENABLE_THREAD_SAFETY
+				pthread_mutex_unlock(&ssl_config_mutex);
+#endif
 				return -1;
 			}
 
@@ -1176,6 +1192,9 @@ initialize_SSL(PGconn *conn)
 				ENGINE_free(conn->engine);
 				conn->engine = NULL;
 				free(engine_str);
+#ifdef ENABLE_THREAD_SAFETY
+				pthread_mutex_unlock(&ssl_config_mutex);
+#endif
 				return -1;
 			}
 			if (SSL_use_PrivateKey(conn->ssl, pkey) != 1)
@@ -1190,6 +1209,9 @@ initialize_SSL(PGconn *conn)
 				ENGINE_free(conn->engine);
 				conn->engine = NULL;
 				free(engine_str);
+#ifdef ENABLE_THREAD_SAFETY
+				pthread_mutex_unlock(&ssl_config_mutex);
+#endif
 				return -1;
 			}
 
@@ -1242,6 +1264,9 @@ initialize_SSL(PGconn *conn)
 			   libpq_gettext("could not load private key file \"%s\": %s\n"),
 							  fnbuf, err);
 			SSLerrfree(err);
+#ifdef ENABLE_THREAD_SAFETY
+			pthread_mutex_unlock(&ssl_config_mutex);
+#endif
 			return -1;
 		}
 	}
@@ -1256,6 +1281,9 @@ initialize_SSL(PGconn *conn)
 						  libpq_gettext("certificate does not match private key file \"%s\": %s\n"),
 						  fnbuf, err);
 		SSLerrfree(err);
+#ifdef ENABLE_THREAD_SAFETY
+		pthread_mutex_unlock(&ssl_config_mutex);
+#endif
 		return -1;
 	}
 
@@ -1284,6 +1312,9 @@ initialize_SSL(PGconn *conn)
 							  libpq_gettext("could not read root certificate file \"%s\": %s\n"),
 							  fnbuf, err);
 			SSLerrfree(err);
+#ifdef ENABLE_THREAD_SAFETY
+			pthread_mutex_unlock(&ssl_config_mutex);
+#endif
 			return -1;
 		}
 
@@ -1311,6 +1342,9 @@ initialize_SSL(PGconn *conn)
 								  libpq_gettext("SSL library does not support CRL certificates (file \"%s\")\n"),
 								  fnbuf);
 				SSLerrfree(err);
+#ifdef ENABLE_THREAD_SAFETY
+				pthread_mutex_unlock(&ssl_config_mutex);
+#endif
 				return -1;
 #endif
 			}
@@ -1341,10 +1375,16 @@ initialize_SSL(PGconn *conn)
 				printfPQExpBuffer(&conn->errorMessage,
 				libpq_gettext("root certificate file \"%s\" does not exist\n"
 							  "Either provide the file or change sslmode to disable server certificate verification.\n"), fnbuf);
+#ifdef ENABLE_THREAD_SAFETY
+			pthread_mutex_unlock(&ssl_config_mutex);
+#endif
 			return -1;
 		}
 	}
 
+#ifdef ENABLE_THREAD_SAFETY
+	pthread_mutex_unlock(&ssl_config_mutex);
+#endif
 	return 0;
 }