Dear Michael, I've got some concerns regarding your patch:
Michael Tuexen via RT wrote: > I have looked at the patch provided by Daniel. All suggested changes are > OK, but there are two additional things which should be fixed: > > 1. In ssl3_read_n() the argument max is overwritten before used. I don't understand all the details of the code but I'm wondering if there was a reason for overwriting max. I did not write the code but I can imagine that it's not a bug and that max is ignored on purpose. Although I do not know the details. Shouldn't you at least bounds check the parameter max then? If I'm correct then (max >= n) and (max <= rb->len - rb->offset) must be true. From looking at the code I can see that ssl3_read_n() gets called with max=s->s3->rbuf.len which is equal to rb->len. Plus rb->offset is always greater or equal than the variable "align" which is !=0 on some platforms. So in this case max (which is equal to rb->len) is greater than (rb->len - rb->offset) and therefore max is too large in this case. > 2. If additional data is behind a valid DTLS record in the UDP packet, > it is read as an additional record instead of being discarded. RFC 4347 says in 4.1.1.: "Multiple DTLS records may be placed in a single datagram." Did you take this into account? To me it seems like you only read the first DTLS record in a datagram and ignore all remaining records. Regards, Daniel ______________________________________________________________________ OpenSSL Project http://www.openssl.org Development Mailing List [email protected] Automated List Manager [email protected]
