Re: LDAP functions implemented poorly

On 03/01/2007 01:11 AM, Doug Goldstein wrote:

Antony Dovgal wrote:

On 03/01/2007 12:35 AM, Doug Goldstein wrote:

Did you really test it with non-NULL terminated strings? Don't you need to add '\0' manually?

The test is that you run the example code from bug #38819, watch PHP crash. Apply my patch and watch PHP not crash. Fairly simple. My backtrace is identical to the reporter's.

Well, I can't do it myself since I don't even have a LDAP server installed. That's why I asked you the question.

If you read the comments by the OpenLDAP developers in the two bugs referenced they have the same reason for using ldap_get_values_len() instead of ldap_get_values() because it's safer incase the data is non-NULL terminated data. In this case PHP's assumption that it's NULL terminated is flawed since it's crashing since it's extending past the end of it's memory segment. (as visible from bug #38819)

I have no doubts it's true, but the question was: did you really test [the NEW patched version of] the code with non-NULL terminated strings?

If I run the example PHP code from bug #38819, PHP will merrily run off the end of a string into no man's land and crash as per the backtrace in bug #38819. With the patch applied, it does not. That sound clearly like the example PHP code in bug #38819 is testing it with a non-NULL terminated string. I hope this is clear.

Yes, that's perfectly clear, thanks. -- Wbr, Antony Dovgal