Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 62fe410

Browse files
author
Neil Conway
committed
Minor fix for LDAP authentication: if an error occurs, we need to
manually release the LDAP handle via ldap_unbind(). This isn't a significant problem in practice because an error eventually results in exiting the process, but we can cleanup correctly without too much pain. In passing, fix an error in snprintf() usage: the "size" parameter to snprintf() is the size of the destination buffer, including space for the NUL terminator. Also, depending on the value of NAMEDATALEN, the old coding could have allowed for a buffer overflow.
1 parent 76d5667 commit 62fe410

File tree

1 file changed

+13
-9
lines changed

1 file changed

+13
-9
lines changed

src/backend/libpq/auth.c

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/libpq/auth.c,v 1.145 2006/10/06 17:13:59 petere Exp $
11+
* $PostgreSQL: pgsql/src/backend/libpq/auth.c,v 1.146 2006/11/06 01:27:52 neilc Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -716,11 +716,11 @@ CheckLDAPAuth(Port *port)
716716
char prefix[128];
717717
char suffix[128];
718718
LDAP *ldap;
719-
int ssl = 0;
719+
bool ssl = false;
720720
int r;
721721
int ldapversion = LDAP_VERSION3;
722722
int ldapport = LDAP_PORT;
723-
char fulluser[128];
723+
char fulluser[NAMEDATALEN + 256 + 1];
724724

725725
if (!port->auth_arg || port->auth_arg[0] == '\0')
726726
{
@@ -750,7 +750,7 @@ CheckLDAPAuth(Port *port)
750750
"ldaps://%127[^:]:%i/%127[^;];%127[^;];%127s",
751751
server, &ldapport, basedn, prefix, suffix);
752752
if (r >= 3)
753-
ssl = 1;
753+
ssl = true;
754754
}
755755
if (r < 3)
756756
{
@@ -766,7 +766,7 @@ CheckLDAPAuth(Port *port)
766766
"ldaps://%127[^/]/%127[^;];%127[^;];%127s",
767767
server, basedn, prefix, suffix);
768768
if (r >= 2)
769-
ssl = 1;
769+
ssl = true;
770770
}
771771
if (r < 2)
772772
{
@@ -799,8 +799,9 @@ CheckLDAPAuth(Port *port)
799799

800800
if ((r = ldap_set_option(ldap, LDAP_OPT_PROTOCOL_VERSION, &ldapversion)) != LDAP_SUCCESS)
801801
{
802+
ldap_unbind(ldap);
802803
ereport(LOG,
803-
(errmsg("could not set LDAP protocol version: error code %d", r)));
804+
(errmsg("could not set LDAP protocol version: error code %d", r)));
804805
return STATUS_ERROR;
805806
}
806807

@@ -827,35 +828,38 @@ CheckLDAPAuth(Port *port)
827828
* should never happen since we import other files from
828829
* wldap32, but check anyway
829830
*/
831+
ldap_unbind(ldap);
830832
ereport(LOG,
831833
(errmsg("could not load wldap32.dll")));
832834
return STATUS_ERROR;
833835
}
834836
_ldap_start_tls_sA = (__ldap_start_tls_sA) GetProcAddress(ldaphandle, "ldap_start_tls_sA");
835837
if (_ldap_start_tls_sA == NULL)
836838
{
839+
ldap_unbind(ldap);
837840
ereport(LOG,
838841
(errmsg("could not load function _ldap_start_tls_sA in wldap32.dll"),
839842
errdetail("LDAP over SSL is not supported on this platform.")));
840843
return STATUS_ERROR;
841844
}
842845

843846
/*
844-
* Leak ldaphandle on purpose, because we need the library to stay
847+
* Leak LDAP handle on purpose, because we need the library to stay
845848
* open. This is ok because it will only ever be leaked once per
846849
* process and is automatically cleaned up on process exit.
847850
*/
848851
}
849852
if ((r = _ldap_start_tls_sA(ldap, NULL, NULL, NULL, NULL)) != LDAP_SUCCESS)
850853
#endif
851854
{
855+
ldap_unbind(ldap);
852856
ereport(LOG,
853-
(errmsg("could not start LDAP TLS session: error code %d", r)));
857+
(errmsg("could not start LDAP TLS session: error code %d", r)));
854858
return STATUS_ERROR;
855859
}
856860
}
857861

858-
snprintf(fulluser, sizeof(fulluser) - 1, "%s%s%s",
862+
snprintf(fulluser, sizeof(fulluser), "%s%s%s",
859863
prefix, port->user_name, suffix);
860864
fulluser[sizeof(fulluser) - 1] = '\0';
861865

0 commit comments

Comments
 (0)