On Fri, Nov 3, 2017 at 12:57 PM, Thomas Munro
<thomas.mu...@enterprisedb.com> wrote:
> 1.  If you set up a pg_hba.conf with a URL that lacks a base DN or
> hostname, hba.c will segfault on startup when it tries to pstrdup a
> null pointer.  Examples: ldapurl="ldap://localhost"; and
> ldapurl="ldap://";.
> 2.  If we fail to bind but have no binddn configured, we'll pass NULL
> to ereport (snprint?) for %s, which segfaults on some libc
> implementations.  That crash requires more effort to reproduce but you
> can see pretty clearly a few lines above in auth.c that it can be
> NULL.  (I'm surprised Coverity didn't complain about that.  Maybe it
> can't see this code due to macros.)

Good question. Indeed Coverity did not complain here, perhaps because
the compiled build is not using openldap?

> Please see attached.

Oops. So...

-        hbaline->ldapserver = pstrdup(urldata->lud_host);
+        if (urldata->lud_host)
+            hbaline->ldapserver = pstrdup(urldata->lud_host);
This prevents the backend to blow up on ldap://.

-       hbaline->ldapbasedn = pstrdup(urldata->lud_dn);
+       if (urldata->lud_dn)
+           hbaline->ldapbasedn = pstrdup(urldata->lud_dn);
And this prevents the crash on ldap://localhost.

-                            port->hba->ldapbinddn, port->hba->ldapserver,
+                            port->hba->ldapbinddn ? port->hba->ldapbinddn : "",
+                            port->hba->ldapserver,
ldapserver should never be NULL thanks to the check on
MANDATORY_AUTH_ARG in parse_hba_line(), still I would tend to be
maniak and do the same check as for ldapbinddn. That feels safer
thinking long-term.

Please note that I have added as well an entry in the next CF to avoid
that bug falling into oblivion:

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to