postgres.git / summary / log / commit / refs

commit    3a3ee655c3e1ce8fce26698bfd9601d85947da06
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Sat Jan 24 18:05:58 2015 +0000

    Replace a bunch more uses of strncpy() with safer coding.
    
    strncpy() has a well-deserved reputation for being unsafe, so make an
    effort to get rid of nearly all occurrences in HEAD.
    
    A large fraction of the remaining uses were passing length less than or
    equal to the known strlen() of the source, in which case no null-padding
    can occur and the behavior is equivalent to memcpy(), though doubtless
    slower and certainly harder to reason about.  So just use memcpy() in
    these cases.
    
    In other cases, use either StrNCpy() or strlcpy() as appropriate (depending
    on whether padding to the full length of the destination buffer seems
    useful).
    
    I left a few strncpy() calls alone in the src/timezone/ code, to keep it
    in sync with upstream (the IANA tzcode distribution).  There are also a
    few such calls in ecpg that could possibly do with more analysis.
    
    AFAICT, none of these changes are more than cosmetic, except for the four
    occurrences in fe-secure-openssl.c, which are in fact buggy: an overlength
    source leads to a non-null-terminated destination buffer and ensuing
    misbehavior.  These don't seem like security issues, first because no stack
    clobber is possible and second because if your values of sslcert etc are
    coming from untrusted sources then you've got problems way worse than this.
    Still, it's undesirable to have unpredictable behavior for overlength
    inputs, so back-patch those four changes to all active branches.


 src/interfaces/libpq/fe-secure.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index bc6c0fc4420..09cffe46afb 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -1073,7 +1073,7 @@ initialize_SSL(PGconn *conn)
 
 	/* Read the client certificate file */
 	if (conn->sslcert && strlen(conn->sslcert) > 0)
-		strncpy(fnbuf, conn->sslcert, sizeof(fnbuf));
+		strlcpy(fnbuf, conn->sslcert, sizeof(fnbuf));
 	else if (have_homedir)
 		snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_CERT_FILE);
 	else
@@ -1264,7 +1264,7 @@ initialize_SSL(PGconn *conn)
 #endif   /* USE_SSL_ENGINE */
 		{
 			/* PGSSLKEY is not an engine, treat it as a filename */
-			strncpy(fnbuf, conn->sslkey, sizeof(fnbuf));
+			strlcpy(fnbuf, conn->sslkey, sizeof(fnbuf));
 		}
 	}
 	else if (have_homedir)
@@ -1327,7 +1327,7 @@ initialize_SSL(PGconn *conn)
 	 * verification after the connection has been completed.
 	 */
 	if (conn->sslrootcert && strlen(conn->sslrootcert) > 0)
-		strncpy(fnbuf, conn->sslrootcert, sizeof(fnbuf));
+		strlcpy(fnbuf, conn->sslrootcert, sizeof(fnbuf));
 	else if (have_homedir)
 		snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CERT_FILE);
 	else
@@ -1365,7 +1365,7 @@ initialize_SSL(PGconn *conn)
 		if ((cvstore = SSL_CTX_get_cert_store(SSL_context)) != NULL)
 		{
 			if (conn->sslcrl && strlen(conn->sslcrl) > 0)
-				strncpy(fnbuf, conn->sslcrl, sizeof(fnbuf));
+				strlcpy(fnbuf, conn->sslcrl, sizeof(fnbuf));
 			else if (have_homedir)
 				snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CRL_FILE);
 			else


[parent: a113a66a7a7d]