hallvard@OpenLDAP.org wrote:
Update of /repo/OpenLDAP/pkg/ldap/libraries/libldap
Modified Files: url.c 1.103 -> 1.104
Log Message: Declare enough buffer space for out-of-range URL port numbers
It would have been better simply to never accept out-of-range port numbers. lud_port should have been an unsigned short instead of int. Or just test for the correct range on assignment and return an error as necessary.
Howard Chu writes:
hallvard@OpenLDAP.org wrote:
url.c 1.103 -> 1.104 Declare enough buffer space for out-of-range URL port numbers
It would have been better simply to never accept out-of-range port numbers. lud_port should have been an unsigned short instead of int. Or just test for the correct range on assignment and return an error as necessary.
Good point. Not quite sure if we can expect port numbers to be < 65536 though. (Some OS could have a non-TCP/IP network protocol and an API with larger port numbers.)
BTW, has IPv6 only 16-bit ports? A brief browse suggests yes; I was a bit surprised. I can imagine that might not scale well in the future.
Anyway, I don't think this is worth breaking binary compatibility for now, and I just noticed that this is in desc2str_len() which does not even produce a string, it just checks its length. So I suggest
if ( u->lud_port ) { /* ":port" or "host:port" */ unsigned p = u->lud_port; if ( p > 65535 ) return -1; len += (p > 999 ? 5 + (p > 9999) : p > 99 ? 4 : 2 + (p > 9)); ...
Hallvard B Furuseth wrote:
Howard Chu writes:
hallvard@OpenLDAP.org wrote:
url.c 1.103 -> 1.104 Declare enough buffer space for out-of-range URL port numbers
It would have been better simply to never accept out-of-range port numbers. lud_port should have been an unsigned short instead of int. Or just test for the correct range on assignment and return an error as necessary.
Good point. Not quite sure if we can expect port numbers to be < 65536 though. (Some OS could have a non-TCP/IP network protocol and an API with larger port numbers.)
But LDAP is only defined over TCP, so this wouldn't matter unless/until something other than TCP comes along with an LDAP spec.
BTW, has IPv6 only 16-bit ports? A brief browse suggests yes; I was a bit surprised. I can imagine that might not scale well in the future.
Yes, ports are part of TCP/UDP, not the IP layer. IPv6 just expands the host address, doesn't change the semantics of TCP or UDP.
Anyway, I don't think this is worth breaking binary compatibility for now, and I just noticed that this is in desc2str_len() which does not even produce a string, it just checks its length. So I suggest
if ( u->lud_port ) { /* ":port" or "host:port" */ unsigned p = u->lud_port; if ( p > 65535 ) return -1; len += (p > 999 ? 5 + (p > 9999) : p > 99 ? 4 : 2 + (p > 9)); ...
That would be fine.
-
Hallvard B Furuseth -
Howard Chu