Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:28287
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 64.233.184.237 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=beta;
        h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references;
        b=Z+FvqtmJTZvIcNQnD14sg21MaR6doBz8C+KYLdH63N1+KJ7xk0gbxjSwzOVvwi2vnQhKkRsCIqdQkMwWs4kpe9k2/wCsTeaIbpVZCmjD8d3UXRVr3/MvBFM/HQ1mJTn7bTskn1UPoe02DlJO5jm4Q0q8W1u0Sdt8RA1I4nL1aUk=
Message-ID: <4e89b4260703070513i186cd3dal49e20bac12305af0@mail.gmail.com>
Date: Wed, 7 Mar 2007 08:13:17 -0500
To: "Doug Goldstein" <cardoe@gentoo.org>
Cc: internals@lists.php.net
In-Reply-To: <45EE5C85.5030409@gentoo.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
References: <40059.216.155.111.10.1172694790.squirrel@webmail.cardoe.com>
	 <4e89b4260703062218v73c3655agdd72b5b7bfdfe41f@mail.gmail.com>
	 <45EE5C85.5030409@gentoo.org>
Subject: Re: [PHP-DEV] LDAP functions implemented poorly
From: kingwez@gmail.com ("Wez Furlong")

What I meant was, make sure that ldap_get_values_len() is also present
in the LDAP libraries on Solaris and Windows.
I've checked the docs on those platforms, and they both have those APIs.
The reason I bring it up is because those 3 implementations have 98%
the same API but some differences in some areas.

Anyhow, it appears safe; carry on :)

--Wez.

On 3/7/07, Doug Goldstein <cardoe@gentoo.org> wrote:
> Wez Furlong wrote:
> Considering my changes just call ldap_get_values_len() instead of
> ldap_get_values(), both of which were implemented on all platforms and
> arches (i.e. no IFDEF sections) then I assume if ldap_get_values_len()
> which appears to have existed and been used in PHP code since the start
> of the extension worked... then using it instead of ldap_get_values()
> should still work. as per the IETF spec for LDAP C API, the
> ldap_get_values_len() function basically returns you a char * as well as
> the length of the data. Instead of just returning you the char * and
> relying on your code to call strlen(). Which when it comes to binary
> data is not safe.
>
> So it shouldn't affect what LDAP server is being used.
>
>
> >
> > --Wez.
> >
> > On 2/28/07, Doug Goldstein <cardoe@gentoo.org> wrote:
> >> Referencing Bug #38819 & Bug #40671
> >> http://bugs.php.net/bug.php?id=38819
> >> http://bugs.php.net/bug.php?id=40671
> >>
> >> Essentially I looked through the above mentioned bug, the bugs opened
> >> with OpenLDAP developers, and then reviewed ext/ldap/ldap.c and it
> >> appears the API calls made by PHP are not necessarily the safest ways to
> >> write the PHP wrapper functions. Based on tony2001@php.net's comment
> >> that the LDAP module is unmaintained I went ahead and made some
> >> changes.
> >>
> >> If you read OpenLDAP's API and comments by OpenLDAP Core Developers,
> >> available at:
> >>
> >> http://www.openldap.org/its/index.cgi/Build?id=4690;selectid=4690
> >> http://www.openldap.org/software/man.cgi?query=ldap_get_values&sektion=3
> >> &apropos=0&manpath=OpenLDAP+2.1-Release
> >>
> >> (Notice I went with OpenLDAP 2.1 docs to quell PHP's urge for backwards
> >> compatibility)
> >>
> >> The functions char **ldap_get_values(ld, entry, attr) and struct berval
> >> **ldap_get_values_len(ld, entry, attr) are essentially inter-changeable.
> >> The big difference being that the berval struct provides you with a char
> >> * and the size_t of the data. Rather then just a char * that you then
> >> have to strlen() which will result in problems if the returned data is
> >> not NULL terminated data. PHP's internal functions make the mistake of
> >> assuming all data will be string data (NULL terminated char *) data,
> >> which is the cause of the crash in bug #38819.
> >>
> >> The patch attached removes all of those assumptions and uses
> >> ldap_get_values_len() and uses the length provided back by the structure
> >> to feed add_index_stringl() instead of using add_index_string() which
> >> will call it's own strlen() on the provided data.
> >>
> >> This patch also removes ldap_get_values() as a PHP function and makes it
> >> an alias of ldap_get_values_len() since there's no difference and the
> >> same data can be returned, it's just a safer version.
> >>
> >> The attached patch fixes the test case provided in bug #38819.
> >>
> >> Referencing for my own purposes:
> >> http://bugs.gentoo.org/show_bug.cgi?id=133467
> >> --
> >> PHP Internals - PHP Runtime Development Mailing List
> >> To unsubscribe, visit: http://www.php.net/unsub.php
> >>
> >>
> >
>
>
> --
> Doug Goldstein <cardoe@gentoo.org>
> http://dev.gentoo.org/~cardoe/
>
>
>