Hi --
I am developing a new Apache 2.0 module and I have encountered what I think
to be a bug in mod_ssl. I have been unable to find any reports of a similar
problem. I think this is because I am using AP_MODE_SPECULATIVE, which is
a bit unusual, so maybe no one else has run into this case yet.
Brief description of the problem:
The problem is related to the use of both AP_MODE_SPECULATIVE and mod_ssl.
Under certain conditions, ssl_engine_io.c:ssl_io_input_read() and its helper
function char_buffer_read() might not handle inctx-cbuf properly. In
particular, this can lead to the inctx-cbuf.value pointer being assigned an
incorrect value, after which Apache may segmentation fault.
Software Environment:
I am running Apache 2.0.4[56] and using the mod_ssl that comes with it.
The compile and runtime environments that I have been working with are
Redhat 2.4.18-3 and FreeBSD 4.5. I am using the standard system development
tools etc.
I am configuring Apache like so:
% ./configure --prefix=/usr/local/apache2-2.0.45 --with-module=aaa:auth_dacs
\
--enable-ssl
% ./httpd -l
Compiled in modules: core.c mod_access.c mod_auth.c mod_include.c \
mod_log_config.c mod_env.c mod_setenvif.c mod_ssl.c prefork.c \
http_core.c mod_mime.c mod_auth_dacs.c mod_status.c mod_autoindex.c \
mod_asis.c mod_cgi.c mod_negotiation.c mod_dir.c mod_imap.c mod_actions.c \
mod_userdir.c mod_alias.c mod_so.c
mod_auth_dacs.c is my new module. The only thing unusual about it, I think,
is that it makes the following call:
rv = ap_get_brigade(r-input_filters, bb, AP_MODE_SPECULATIVE,
APR_BLOCK_READ, need);
Conditions:
The conditions under which this happens are browser dependent. It doesn't
seem to ever happen with Mozilla 1.3 or Netscape 4.78 but it does happen
reproducibly with IE 6.0 and Netscape Navigator 3.01. I think this is
related to the way they do SSL I/O because while debugging I see that the
SSL buffer processing in mod_ssl is slightly different with IE 6.0.
To trigger the bug, I invoke a POST method using https from IE 6.0, which
leads mod_auth_dacs to make the function call that appears above.
The bug does not seem to be present when SSL is not used.
Detailed description of the problem:
The problem seems to occur if an AP_MODE_SPECULATIVE mode read operation
is done when ssl_io_input_read() already has input buffered in inctx-cbuf.
Here is ssl_engine_io.c:char_buffer_read():
static int char_buffer_read(char_buffer_t *buffer, char *in, int inl)
{
if (!buffer-length) {
return 0;
}
if (buffer-length inl) {
/* we have have enough to fill the caller's buffer */
memcpy(in, buffer-value, inl);
buffer-value += inl;
buffer-length -= inl;
}
else {
/* swallow remainder of the buffer */
memcpy(in, buffer-value, buffer-length);
inl = buffer-length;
buffer-value = NULL;
buffer-length = 0;
}
return inl;
}
If there is not enough buffered input, char_buffer_read() sets
buffer-value to NULL. Then, when it returns to ssl_io_input_read():
if ((bytes = char_buffer_read(inctx-cbuf, buf, wanted))) {
*len = bytes;
if (inctx-mode == AP_MODE_SPECULATIVE) {
/* We want to rollback this read. */
inctx-cbuf.value -= bytes;
inctx-cbuf.length += bytes;
return APR_SUCCESS;
}
it will do inctx-cbuf.value -= bytes which sets cbuf.value to an invalid
pointer. When the pointer is dereferenced later, a segmentation fault
occurs. So at the very least, it would seem that the code should always
avoid doing this.
Also, I found that in char_buffer_read(), in and buffer-value may
overlap, so memmove() should probably be used instead of memcpy().
Proposed fix:
I have not tested this heavily, but it does seem to solve the problem in
my test case (and my module then works with IE 6.0 and Navigator 3.01, as
well as Mozilla and Netscape 4.78).
I changed ssl_io_input_read() like so:
if ((bytes = char_buffer_read(inctx, buf, wanted))) {
*len = bytes;
if (inctx-mode == AP_MODE_SPECULATIVE) {
return APR_SUCCESS;
}
And I changed char_buffer_read() like so:
static int char_buffer_read(bio_filter_in_ctx_t *inctx, char *in, int inl)
{
char_buffer_t *buffer = inctx-cbuf;
if (!buffer-length) {
return 0;
}
if (buffer-length inl) {
/* we have have enough to fill the caller's buffer */
memmove(in, buffer-value, inl);
buffer-value += inl;
buffer-length -= inl;
}
else {
/* swallow remainder of the buffer */
memmove(in, buffer-value, buffer-length);
inl = buffer-length;
if (inctx-mode == AP_MODE_SPECULATIVE) {
buffer-value = in;
}
else {
buffer-value = NULL;
