CVE-2014-5336: Improper Input Validation (CWE-20)
value: Monkey HTTP Server before 1.5.3, when the File Descriptor Table (FDT) is enabled and custom error messages are set, allows remote attackers to cause a denial of service (file descriptor consumption) via an HTTP request that triggers an error message.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
Published: 2014-08-26T14:55Z
Repository: https://github.com/monkey/monkey
Language: C
Files changed: include/monkey/mk_request.h, src/mk_request.c, src/mk_vhost.c
Commit date: 2014-08-16 11:55:28 -0600
Commit message: Request: new request session flag to mark those files opened by FDT

This patch aims to fix a potential DDoS problem that can be caused
in the server quering repetitive non-existent resources.

When serving a static file, the core use Vhost FDT mechanism, but if
it sends a static error page it does a direct open(2). When closing
the resources for the same request it was just calling mk_vhost_close()
which did not clear properly the file descriptor.

This patch adds a new field on the struct session_request called 'fd_is_fdt',
which contains MK_TRUE or MK_FALSE depending of how fd_file was opened.

Thanks to Matthew Daley <mattd@bugfuzz.com> for report and troubleshoot this
problem.

Signed-off-by: Eduardo Silva <eduardo@monkey.io>

Diff: diff --git a/include/monkey/mk_request.h b/include/monkey/mk_request.h
index 40acbe72..9d0213ff 100644
--- a/include/monkey/mk_request.h
+++ b/include/monkey/mk_request.h
@@ -157,12 +157,22 @@ struct session_request
 
     /*-Connection-*/
     long port;
     /*------------*/
 
-    /* file descriptors */
+    /*
+     * Static file file descriptor: the following twp fields represents an
+     * opened file in the file system and a flag saying which mechanism
+     * was used to open it.
+     *
+     *  - fd_file  : common file descriptor
+     *  - fd_is_fdt: set to MK_TRUE if fd_file was opened using Vhost FDT, or
+     *               MK_FALSE for the opposite case.
+     */
     int fd_file;
+    int fd_is_fdt;
+
 
     int headers_len;
 
     /*----First header of client request--*/
     int method;
diff --git a/src/mk_request.c b/src/mk_request.c
index adce9ff1..94ffdf8e 100644
--- a/src/mk_request.c
+++ b/src/mk_request.c
@@ -97,11 +97,16 @@ static inline void mk_request_init(struct session_request *request)
 }
 
 void mk_request_free(struct session_request *sr)
 {
     if (sr->fd_file > 0) {
-        mk_vhost_close(sr);
+        if (sr->fd_is_fdt == MK_TRUE) {
+            mk_vhost_close(sr);
+        }
+        else {
+            close(sr->fd_file);
+        }
     }
 
     if (sr->headers.location) {
         mk_mem_free(sr->headers.location);
     }
@@ -839,11 +844,12 @@ int mk_request_error(int http_status, struct client_session *cs,
             fd = open(entry->real_path, config->open_flags);
             if (fd == -1) {
                 break;
             }
 
-            sr->fd_file = fd;
+            sr->fd_file   = fd;
+            sr->fd_is_fdt = MK_FALSE;
             sr->bytes_to_send = finfo.size;
             sr->headers.content_length = finfo.size;
             sr->headers.real_length    = finfo.size;
 
             memcpy(&sr->file_info, &finfo, sizeof(struct file_info));
diff --git a/src/mk_vhost.c b/src/mk_vhost.c
index cd89582f..4807faf0 100644
--- a/src/mk_vhost.c
+++ b/src/mk_vhost.c
@@ -218,10 +218,11 @@ static inline int mk_vhost_fdt_open(int id, unsigned int hash,
             hc->readers++;
             ht->av_slots--;
 
             sr->vhost_fdt_id   = id;
             sr->vhost_fdt_hash = hash;
+            sr->fd_is_fdt      = MK_TRUE;
 
             return fd;
         }
     }
 
@@ -260,11 +261,10 @@ static inline int mk_vhost_fdt_close(struct session_request *sr)
         }
         else {
             return 0;
         }
     }
-
     return close(sr->fd_file);
 }
 
 
 int mk_vhost_open(struct session_request *sr)

Vulnerable code:     /* file descriptors */
        mk_vhost_close(sr);
            sr->fd_file = fd;

Fixed code:     /*
     * Static file file descriptor: the following twp fields represents an
     * opened file in the file system and a flag saying which mechanism
     * was used to open it.
     *
     *  - fd_file  : common file descriptor
     *  - fd_is_fdt: set to MK_TRUE if fd_file was opened using Vhost FDT, or
     *               MK_FALSE for the opposite case.
     */
    int fd_is_fdt;

        if (sr->fd_is_fdt == MK_TRUE) {
            mk_vhost_close(sr);
        }
        else {
            close(sr->fd_file);
        }
            sr->fd_file   = fd;
            sr->fd_is_fdt = MK_FALSE;
            sr->fd_is_fdt      = MK_TRUE;