Need feedback for proposed changes in Apache source

[Rai, Pravesh R (STSD)]

Hi,

We are using Apache 2.2.21 with our product in HP. As we all know that during 
some failure operations, Windows OS stores the memory dump as .mdmp & .hdmp 
files. In our case we have observed credentials (in plain text) in those dump 
files, which is a security concern for us.

During our investigation, we found that Apache source uses memcpy() at many 
places, which always leave behind the source string (in this case, credentials 
in plain text) in the memory. Also observed that the destination buffer, if 
bigger than the source buffer, always have remnants of its original content 
after copy/move operations. Such memory locations hold the data for unknown 
longer duration & any exception during the course exposes all these data in the 
dump file.

Have tried to modify few Apache source files, like:

httpd\srclib\apr-util\buckets\apr_brigade.c (diff file w.r.t. to Apache 2.2.21: 
diff_apr_brigade.c.txt)
httpd\modules\ssl\ssl_engine_io.c (diff file w.r.t. to Apache 2.2.21: 
diff_ssl_engine_io.c.txt)

Though the changes are minor & mainly intended to clean the buffer, but so far 
our Security testing team has not found any plain text credentials in any of 
our application dump files. Please go through these changes & let us know your 
views.

Thanks & Regards,
Pravesh

--- apr_brigade_org.c   Fri Jan 27 17:50:42 2012
+++ apr_brigade.c       Tue Dec 13 11:29:47 2011
@@ -250,6 +250,7 @@
          * No, we only copy the data up to their requested size.  -- jre
          */
         memcpy(c, str, str_len);
+               memset(str, 0, str_len);
 
         c += str_len;
         actual += str_len;

--- ssl_engine_io_org.c Fri Jan 27 18:24:34 2012
+++ ssl_engine_io.c     Fri Jan 27 17:58:39 2012
@@ -366,7 +366,9 @@
     else {
         /* swallow remainder of the buffer */
         memmove(in, buffer->value, buffer->length);
-        inl = buffer->length;
+               memset(in + buffer->length, 0, AP_IOBUFSIZE - buffer->length);
+
+               inl = buffer->length;
         buffer->value = NULL;
         buffer->length = 0;
     }
@@ -478,6 +480,10 @@
     /* OpenSSL catches this case, so should we. */
     if (!in)
         return 0;
+
+       // Cleaning up the existing buffer...
+       if(inlen < AP_IOBUFSIZE)
+               memset(in + inlen, 0, AP_IOBUFSIZE - inlen);
 
     /* Abort early if the client has initiated a renegotiation. */
     if (inctx->filter_ctx->config->reneg_state == RENEG_ABORT) {