On 26/04/2014 11:04 PM, Kurt Roeckx via RT wrote: > Libressl has a patch for this at: > http://anoncvs.estpak.ee/cgi-bin/cgit/openbsd-src/commit/lib/libssl?id=cb8b51bf2f6517fe96ab0d20c4d9bba2eef1b67c > > I believe that patch is not really the correct fix. > > My understanding is that "tot" is what is already written, and > that "len" is until where we want to write and so that len should > never be smaller than tot and I think we should instead find out > why len can be smaller then tot and fix that instead.
Actually the code should check that the caller has violated the API requirements (that the buffer and len values in subsequent calls must match the original call when a non-blocking IO reports incomplete) and report an error at that point. Silently truncating the request isn't really the right behaviour. The referenced note from the list at http://marc.info/?l=openssl-dev&m=139809485525663&w=2 <http://marc.info/?l=openssl-dev&m=139809485525663&w=2> makes the circumstances to trigger this clear. i=SSL_write(s,buf,len); /* assuming that the non-blocking handling returns -1 indicating buffer is not yet (completely) written */ /* this is what the API requires - recalling with the same parameters */ i=SSL_write(s,buf,len); /* this is what the caller did ... and that leaves tot > len if less than someval bytes have gone out between the first call and then the rest of the pending bytes make it out in this call then bingo we get the library writing out a large buffer because it isn't checking ... */ i=SSL_write(s,buf,len-someval); The actual effect is that if a user incorrectly calls the API the library will (under the right set of circumstances which are not that unusual but neither the typical context) actually send out beyond the number of bytes the user is expecting to see sent from the buffer because of the n = (len - tot) when tot > len and that is undesirable behaviour and well beyond the 'len' argument of the call - thereby allowing the users incorrect use of the API to turn into their leakage of whatever happens to be beyond the buffer. BUT (and this is the important thing) the circumstances described as required to produce this context get checked in ssl3_write_pending to make sure that len cannot actually be smaller than the previous write. if ((s->s3->wpend_tot > (int)len) The right fix is perhaps to add in the following check ... if ( len < tot) { SSLerr(SSL_F_SSL3_WRITE_BYTES,SSL_R_BAD_LENGTH); return(-1); } I've created a pull request for this at https://github.com/openssl/openssl/pull/83 Thanks, Tim. ______________________________________________________________________ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
