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]
