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 tostrlen()which will result in problems if the returned data is
notNULLterminated 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
Hello.
Thanks for sending the patch here.
Referencing Bug #38819 & Bug #40671
http://bugs.php.net/bug.php?id=38819
http://bugs.php.net/bug.php?id=40671Essentially 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.
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 tostrlen()which will result in problems if the returned data is
notNULLterminated 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.
Wait, I thought the DEPRECATED thing fixed it (I can't test it myself as I don't use LDAP).
If not, then what was it all about?
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 ownstrlen()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.
Some comments regarding the patch:
First of all, why do you add a new function and comment the old one instead of
changing/fixing the old function?
We definitely don't need dead/commented out code in the sources =)
Then, it's a bit too late to change 5.1.6, when 5.2.2 is on it's way, so surely
we would like to see the patches for 5.2.2 instead.
You can get its sources from CVS using PHP_5_2 branch:
cvs -d :pserver:anonymous@cvs.php.net/repository co -r PHP_5_2 php-src
--
Wbr,
Antony Dovgal
Antony Dovgal wrote:
Hello.
Thanks for sending the patch here.Referencing Bug #38819 & Bug #40671
http://bugs.php.net/bug.php?id=38819
http://bugs.php.net/bug.php?id=40671Essentially 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.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 tostrlen()which will result in problems if the returned data is
notNULLterminated 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.Wait, I thought the DEPRECATED thing fixed it (I can't test it myself as I
don't use LDAP).
If not, then what was it all about?
The test case still failed for me. It also does not address the fact that
the usage of the PHP ldap functions is unsafe because they could return a
char * with a non-NULL terminated string and your code will eventually
call strlen() on it. With the changes I have made it knows the length from
the LDAP server and doesn't need to use strlen(). Since
add_index_stringl() doesn't call strlen() since the length is provided.
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 ownstrlen()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.Some comments regarding the patch:
First of all, why do you add a new function and comment the old one
instead of
changing/fixing the old function?
We definitely don't need dead/commented out code in the sources =)
I didn't add a new function. I replaced a function with an alias since the
function is pointlessly there. It's duplicated code and the net result is
just an alias.
Then, it's a bit too late to change 5.1.6, when 5.2.2 is on it's way, so
surely
we would like to see the patches for 5.2.2 instead.
This patch is against 5.1.6 and the 5.2 branch. The code in question has
not been touched since before 5.1.0 was released. I tested it against both
5.2 branch and 5.1.6 prior to uploading it.
You can get its sources from CVS using PHP_5_2 branch:
cvs -d :pserver:anonymous@cvs.php.net/repository co -r PHP_5_2 php-src
You might want to update your instructions to people since the user is
"cvsread" and not "anonymous".
--
Wbr,
Antony Dovgal
Wait, I thought the DEPRECATED thing fixed it (I can't test it myself as I
don't use LDAP).
If not, then what was it all about?The test case still failed for me. It also does not address the fact that
the usage of the PHP ldap functions is unsafe because they could return a
char * with a non-NULL terminated string and your code will eventually
callstrlen()on it. With the changes I have made it knows the length from
the LDAP server and doesn't need to usestrlen(). Since
add_index_stringl() doesn't callstrlen()since the length is provided.
Did you really test it with non-NULL terminated strings?
Don't you need to add '\0' manually?
Some comments regarding the patch:
First of all, why do you add a new function and comment the old one
instead of
changing/fixing the old function?
We definitely don't need dead/commented out code in the sources =)I didn't add a new function. I replaced a function with an alias since the
function is pointlessly there. It's duplicated code and the net result is
just an alias.
No, replace means "add something and delete the original", you added new function and
commented out the original one, which made me wonder why you didn't change the original function instead.
What was the point in creating new function adding an alias?
There is nothing wrong, I'm just curious.
Then, it's a bit too late to change 5.1.6, when 5.2.2 is on it's way, so
surely
we would like to see the patches for 5.2.2 instead.This patch is against 5.1.6 and the 5.2 branch. The code in question has
not been touched since before 5.1.0 was released. I tested it against both
5.2 branch and 5.1.6 prior to uploading it.
Ok then.
You can get its sources from CVS using PHP_5_2 branch:
cvs -d :pserver:anonymous@cvs.php.net/repository co -r PHP_5_2 php-srcYou might want to update your instructions to people since the user is
"cvsread" and not "anonymous".
Oh, that's just my memory.
--
Wbr,
Antony Dovgal
Antony Dovgal wrote:
Wait, I thought the DEPRECATED thing fixed it (I can't test it myself
as I
don't use LDAP).
If not, then what was it all about?The test case still failed for me. It also does not address the fact
that
the usage of the PHP ldap functions is unsafe because they could return
a
char * with a non-NULL terminated string and your code will eventually
callstrlen()on it. With the changes I have made it knows the length
from
the LDAP server and doesn't need to usestrlen(). Since
add_index_stringl() doesn't callstrlen()since the length is provided.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.
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)
Some comments regarding the patch:
First of all, why do you add a new function and comment the old one
instead of
changing/fixing the old function?
We definitely don't need dead/commented out code in the sources =)I didn't add a new function. I replaced a function with an alias since
the
function is pointlessly there. It's duplicated code and the net result
is
just an alias.No, replace means "add something and delete the original", you added new
function and
commented out the original one, which made me wonder why you didn't change
the original function instead.
What was the point in creating new function adding an alias?
There is nothing wrong, I'm just curious.
I removed PHP_FE(ldap_get_values) because it's a pointless function. It's
identical in code to PHP_FE(ldap_get_values_len) so I made it an alias.
Less code to maintain for you guys. Less bloat. Everyone should be happy.
Then, it's a bit too late to change 5.1.6, when 5.2.2 is on it's way,
so
surely
we would like to see the patches for 5.2.2 instead.This patch is against 5.1.6 and the 5.2 branch. The code in question has
not been touched since before 5.1.0 was released. I tested it against
both
5.2 branch and 5.1.6 prior to uploading it.Ok then.
You can get its sources from CVS using PHP_5_2 branch:
cvs -d :pserver:anonymous@cvs.php.net/repository co -r PHP_5_2 php-srcYou might want to update your instructions to people since the user is
"cvsread" and not "anonymous".Oh, that's just my memory.
--
Wbr,
Antony Dovgal
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'sNULL
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?
As far as I understand, if OpenLDAP returns non-NULL terminated string, we'd most likely need
to add the terminator, otherwise we'd end up with non-NULL terminated strings in PHP.
Hence the question.
Do I make myself clear enough?
No, replace means "add something and delete the original", you added new
function and
commented out the original one, which made me wonder why you didn't change
the original function instead.
What was the point in creating new function adding an alias?
There is nothing wrong, I'm just curious.I removed PHP_FE(ldap_get_values) because it's a pointless function. It's
identical in code to PHP_FE(ldap_get_values_len) so I made it an alias.
Less code to maintain for you guys. Less bloat. Everyone should be happy.
Ok, so we got some misunderstanding here..
I removed PHP_FE(ldap_get_values) because it's a pointless function.
The point is that the patch should FIX this function, not REMOVE it.
And it's still there, but commented out (which makes not sense).
Don't get me wrong, I can (and I will) rewrite it myself, but I just
wanted to understand what was the initial idea behind it.
--
Wbr,
Antony Dovgal
I removed PHP_FE(ldap_get_values) because it's a pointless function.
The point is that the patch should FIX this function, not REMOVE it.
And it's still there, but commented out (which makes not sense).
Uhm.. Ignore that, I need to sleep more.
--
Wbr,
Antony Dovgal
Antony Dovgal 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'sNULL
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.
As far as I understand, if OpenLDAP returns non-NULL terminated string,
we'd most likely need
to add the terminator, otherwise we'd end up with non-NULL terminated
strings in PHP.
Hence the question.Do I make myself clear enough?
If you read the PHP API documentation, the API function call
add_index_stringl()
http://php.interec.es/manual/nl/zend-api.add-index-stringl.php
Was created for this explicit purpose. There is no need to tack on a NULL
character since it is automatically happening. If it's not happening, then
it is properly using the size passed to it to always ensure it doesn't
wander off the end of the memory address anywhere. I hope that is clear.
No, replace means "add something and delete the original", you added
new
function and
commented out the original one, which made me wonder why you didn't
change
the original function instead.
What was the point in creating new function adding an alias?
There is nothing wrong, I'm just curious.I removed PHP_FE(ldap_get_values) because it's a pointless function.
It's
identical in code to PHP_FE(ldap_get_values_len) so I made it an alias.
Less code to maintain for you guys. Less bloat. Everyone should be
happy.Ok, so we got some misunderstanding here..
I removed PHP_FE(ldap_get_values) because it's a pointless function.
The point is that the patch should FIX this function, not REMOVE it.
And it's still there, but commented out (which makes not sense).Don't get me wrong, I can (and I will) rewrite it myself, but I just
wanted to understand what was the initial idea behind it.
The implemented PHP_FUNCTION(ldap_get_values) and
PHP_FUNCTION(ldap_get_values_len) differ on in the fact that they call
ldap_get_values() and ldap_get_values_len(). Since the premise behind all
these fixes is to remove the calls to ldap_get_values() and replace them
with ldap_get_values_len() so you ensure that you don't wander off the end
of a string and cause a crash.. that makes PHP_FUNCTION(ldap_get_values)
and PHP_FUNCTION(ldap_get_values_len) identical in code. Line for line
there would be 0 difference. Rather then having a completely duplicated
function and duplicated code (duplicated code is bad), I changed
PHP_FUNCTION(ldap_get_values) to an alias of
PHP_FUNCTION(ldap_get_values_len)
--
Wbr,
Antony Dovgal
Antony Dovgal 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'sNULL
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
Antony Dovgal wrote:
Antony Dovgal 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'sNULL
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.
So any word about this getting merged for PHP 5.2.2?
--
Doug Goldstein cardoe@gentoo.org
http://dev.gentoo.org/~cardoe/
My really really late 2 cents on this.
Please make sure that your changes don't make the extension depend on
OpenLDAP. Solaris and Windows LDAP implementations are close but not
totally the same as OpenLDAP.
I haven't looked at your patches and probably won't have time to do
so; I'm merely doing a drive-by peek at the recent traffic on the
lists, so forgive me if you've already taken this into consideration
:)
--Wez.
Referencing Bug #38819 & Bug #40671
http://bugs.php.net/bug.php?id=38819
http://bugs.php.net/bug.php?id=40671Essentially 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 tostrlen()which will result in problems if the returned data is
notNULLterminated 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 ownstrlen()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
Wez Furlong wrote:
My really really late 2 cents on this.
Please make sure that your changes don't make the extension depend on
OpenLDAP. Solaris and Windows LDAP implementations are close but not
totally the same as OpenLDAP.I haven't looked at your patches and probably won't have time to do
so; I'm merely doing a drive-by peek at the recent traffic on the
lists, so forgive me if you've already taken this into consideration
:)
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.
Referencing Bug #38819 & Bug #40671
http://bugs.php.net/bug.php?id=38819
http://bugs.php.net/bug.php?id=40671Essentially 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 tostrlen()which will result in problems if the returned data is
notNULLterminated 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 ownstrlen()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
--
Doug Goldstein cardoe@gentoo.org
http://dev.gentoo.org/~cardoe/
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.
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 callstrlen(). Which when it comes to binary
data is not safe.So it shouldn't affect what LDAP server is being used.
--Wez.
Referencing Bug #38819 & Bug #40671
http://bugs.php.net/bug.php?id=38819
http://bugs.php.net/bug.php?id=40671Essentially 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 tostrlen()which will result in problems if the returned data is
notNULLterminated 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 ownstrlen()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--
Doug Goldstein cardoe@gentoo.org
http://dev.gentoo.org/~cardoe/