Fix write-past-buffer-end in ldapServiceLookup().

tglsfdc · tglsfdc · commit 30cf86fdf49e · 2011-05-12T11:57:05.000-04:00
The code to assemble ldap_get_values_len's output into a single string
wrote the terminating null one byte past where it should.  Fix that,
and make some other cosmetic adjustments to make the code a trifle more
readable and more in line with usual Postgres coding style.

Also, free the "result" string when done with it, to avoid a permanent
memory leak.

Bug report and patch by Albe Laurenz, cosmetic adjustments by me.
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
@@ -3339,25 +3339,26 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
 		return 1;
 	}
 
-	/* concatenate values to a single string */
-	for (size = 0, i = 0; values[i] != NULL; ++i)
+	/* concatenate values into a single string with newline terminators */
+	size = 1;					/* for the trailing null */
+	for (i = 0; values[i] != NULL; i++)
 		size += values[i]->bv_len + 1;
-	if ((result = malloc(size + 1)) == NULL)
+	if ((result = malloc(size)) == NULL)
 	{
 		printfPQExpBuffer(errorMessage,
 						  libpq_gettext("out of memory\n"));
 		ldap_value_free_len(values);
 		ldap_unbind(ld);
 		return 3;
 	}
-	for (p = result, i = 0; values[i] != NULL; ++i)
+	p = result;
+	for (i = 0; values[i] != NULL; i++)
 	{
-		strncpy(p, values[i]->bv_val, values[i]->bv_len);
+		memcpy(p, values[i]->bv_val, values[i]->bv_len);
 		p += values[i]->bv_len;
 		*(p++) = '\n';
-		if (values[i + 1] == NULL)
-			*(p + 1) = '\0';
 	}
+	*p = '\0';
 
 	ldap_value_free_len(values);
 	ldap_unbind(ld);
@@ -3386,6 +3387,7 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
 					printfPQExpBuffer(errorMessage, libpq_gettext(
 					"missing \"=\" after \"%s\" in connection info string\n"),
 									  optname);
+					free(result);
 					return 3;
 				}
 				else if (*p == '=')
@@ -3404,6 +3406,7 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
 					printfPQExpBuffer(errorMessage, libpq_gettext(
 					"missing \"=\" after \"%s\" in connection info string\n"),
 									  optname);
+					free(result);
 					return 3;
 				}
 				break;
@@ -3467,6 +3470,7 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
 				printfPQExpBuffer(errorMessage,
 						 libpq_gettext("invalid connection option \"%s\"\n"),
 								  optname);
+				free(result);
 				return 1;
 			}
 			optname = NULL;
@@ -3475,6 +3479,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
 		oldstate = state;
 	}
 
+	free(result);
+
 	if (state == 5 || state == 6)
 	{
 		printfPQExpBuffer(errorMessage, libpq_gettext(
