[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#5318) response msgid <= 0 mishandled in libldap

h.b.furuseth@usit.uio.no wrote:
> Full_Name: Hallvard B Furuseth
> Version: HEAD
> OS:
> URL:
> Submission from: (NULL) (
> Submitted by: hallvard
> libraries/libldap/result.c:try_read1msg() accesses 'lr' uninitialized
> if 'id' (message ID) from line 577's 'ber_get_int( ber,&id )' is<= 0.
> I'm not sure if the client should terminate the connection when it
> receives message id<  0, or if it should just toss the response like
> it does with unknown message IDs.

RFC4511 isn't really explicit here, although it does say that the connection 
should be dropped for unparsable messages. Anyway, I've patched HEAD to toss 
the messages for now.

> With message ID 0, the code reaches this statement with 'lr' uninitialized:
> 		"read1msg: ld %p msgid %ld message type %s\n",
> 		(void *)ld, (long)lr->lr_msgid, ldap_int_msgtype2str( tag ) );
> As far as I can tell, normally lr->lr_msgid == id.  I haven't tracked what
> those values are with LDAP_CONNECTIONLESS at the 'nextresp2:' label.

For CONNECTIONLESS, it can only jump back there because there were multiple 
responses to the current request. So the lr is the same as for the first 
response. No problem there. (Remember this is all within a single datagram. 
Responses to multiple different requests cannot be interleaved at this point.)

> A 700-line function with 5 labels, yuck.
> Anyway, I wonder why taht statement and the statement below:
> 	if ( id == 0 ) {
> doesn't use the same value, either id or lr->lr_msgid for both.

   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/