Apache2: segmentation fault if writing to audit log fails

[edieterich]

If SecAuditLogType is set to Concurrent and apr_file_write_full() in
apache2/msc_logging.c, sec_auditlog_write() fails, a segmentation
fault occurs in sec_audit_logger() when apr_file_close() is called
because msr->new_auditlog_fd is NULL.

Here's the idea for a patch:

--- modsecurity-apache_2.7.7.orig/apache2/msc_logging.c 2013-11-21 16:23:11.000000000 +0100
+++ modsecurity-apache_2.7.7/apache2/msc_logging.c      2014-02-17 11:52:24.627656039 +0100
@@ -52,13 +52,19 @@
         msr_log(msr, 1, "Audit log: Failed writing (requested %" APR_SIZE_T_FMT
             " bytes, written %" APR_SIZE_T_FMT ")", nbytes, nbytes_written);

+        /* Concurrent logging: don't leak file handle. */
+        if (msr->txcfg->auditlog_type == AUDITLOG_CONCURRENT) {
+            apr_file_close(msr->new_auditlog_fd);
+        }
+
         /* Set to NULL to prevent more than one error message on
          * out-of-disk-space events and to prevent further attempts
          * to write to the same file in this request.
          *
          * Note that, as we opened the file through the pool mechanism of
          * the APR, we do not need to close the file here. It will be closed
-         * automatically at the end of the request.
+         * automatically at the end of the request (applies only to serial
+         * logging).
          */
         msr->new_auditlog_fd = NULL;

@@ -1181,7 +1187,10 @@

     /* From here on only concurrent-style processing. */

-    apr_file_close(msr->new_auditlog_fd);
+    /* Fd might already be closed after write failure. */
+    if (msr->new_auditlog_fd) {
+        apr_file_close(msr->new_auditlog_fd);
+    }

     /* Write an entry to the index file */

[edieterich]

I created a pull request (see #668) and close this issue.