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

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
index 1b409d1..6648753 100644 (file)
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3596,10 +3596,11 @@ 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"));
@@ -3607,14 +3608,14 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
                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);
@@ -3643,6 +3644,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 == '=')
@@ -3661,6 +3663,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;
@@ -3724,6 +3727,7 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
                                printfPQExpBuffer(errorMessage,
                                                 libpq_gettext("invalid connection option \"%s\"\n"),
                                                                  optname);
+                               free(result);
                                return 1;
                        }
                        optname = NULL;
@@ -3732,6 +3736,8 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
                oldstate = state;
        }
 
+       free(result);
+
        if (state == 5 || state == 6)
        {
                printfPQExpBuffer(errorMessage, libpq_gettext(