You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by tr...@apache.org on 2015/04/25 13:43:04 UTC

svn commit: r1676013 - /apr/apr/trunk/file_io/win32/pipe.c

Author: trawick
Date: Sat Apr 25 11:43:04 2015
New Revision: 1676013

URL: http://svn.apache.org/r1676013
Log:
SECURITY: CVE-2015-1829 (cve.mitre.org)
APR applications using APR named pipe support on Windows can be 
vulnerable to a pipe squatting attack from a local process; the extent
of the vulnerability, when present, depends on the application.
Initial analysis and report was provided by John Hernandez of Casaba 
Security via HP SSRT Security Alert.

Submitted by: ylavic

Modified:
    apr/apr/trunk/file_io/win32/pipe.c

Modified: apr/apr/trunk/file_io/win32/pipe.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/win32/pipe.c?rev=1676013&r1=1676012&r2=1676013&view=diff
==============================================================================
--- apr/apr/trunk/file_io/win32/pipe.c (original)
+++ apr/apr/trunk/file_io/win32/pipe.c Sat Apr 25 11:43:04 2015
@@ -18,6 +18,7 @@
 #include "apr_file_io.h"
 #include "apr_general.h"
 #include "apr_strings.h"
+#include "apr_escape.h"
 #if APR_HAVE_ERRNO_H
 #include <errno.h>
 #endif
@@ -82,7 +83,6 @@ APR_DECLARE(apr_status_t) apr_file_pipe_
     static unsigned long id = 0;
     DWORD dwPipeMode;
     DWORD dwOpenMode;
-    char name[50];
 
     sa.nLength = sizeof(sa);
 
@@ -127,8 +127,26 @@ APR_DECLARE(apr_status_t) apr_file_pipe_
     (void) apr_pollset_create(&(*out)->pollset, 1, p, 0);
 #endif
     if (apr_os_level >= APR_WIN_NT) {
+        char rand[8];
+        int pid = getpid();
+#define FMT_PIPE_NAME "\\\\.\\pipe\\apr-pipe-%x.%lx."
+        /*                                    ^   ^ ^
+         *                                  pid   | |
+         *                                        | |
+         *                                       id |
+         *                                          |
+         *                        hex-escaped rand[8] (16 bytes)
+         */
+        char name[sizeof FMT_PIPE_NAME + 2 * sizeof(pid)
+                                       + 2 * sizeof(id)
+                                       + 2 * sizeof(rand)];
+        apr_size_t pos;
+
         /* Create the read end of the pipe */
         dwOpenMode = PIPE_ACCESS_INBOUND;
+#ifdef FILE_FLAG_FIRST_PIPE_INSTANCE
+        dwOpenMode |= FILE_FLAG_FIRST_PIPE_INSTANCE;
+#endif
         if (blocking == APR_WRITE_BLOCK /* READ_NONBLOCK */
                || blocking == APR_FULL_NONBLOCK) {
             dwOpenMode |= FILE_FLAG_OVERLAPPED;
@@ -136,10 +154,11 @@ APR_DECLARE(apr_status_t) apr_file_pipe_
             (*in)->pOverlapped->hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
             (*in)->timeout = 0;
         }
-
         dwPipeMode = 0;
 
-        sprintf(name, "\\\\.\\pipe\\apr-pipe-%u.%lu", getpid(), id++);
+        apr_generate_random_bytes(rand, sizeof rand);
+        pos = apr_snprintf(name, sizeof name, FMT_PIPE_NAME, pid, id++);
+        apr_escape_hex(name + pos, rand, sizeof rand, 0, NULL);
 
         (*in)->filehand = CreateNamedPipe(name,
                                           dwOpenMode,
@@ -149,6 +168,11 @@ APR_DECLARE(apr_status_t) apr_file_pipe_
                                           65536,        /* nInBufferSize,   */
                                           1,            /* nDefaultTimeOut, */
                                           &sa);
+        if ((*in)->filehand == INVALID_HANDLE_VALUE) {
+            apr_status_t rv = apr_get_os_error();
+            file_cleanup(*in);
+            return rv;
+        }
 
         /* Create the write end of the pipe */
         dwOpenMode = FILE_ATTRIBUTE_NORMAL;
@@ -167,6 +191,12 @@ APR_DECLARE(apr_status_t) apr_file_pipe_
                                       OPEN_EXISTING,   /* dwCreationDisposition   */
                                       dwOpenMode,      /* Pipe attributes         */
                                       NULL);           /* handle to template file */
+        if ((*out)->filehand == INVALID_HANDLE_VALUE) {
+            apr_status_t rv = apr_get_os_error();
+            file_cleanup(*out);
+            file_cleanup(*in);
+            return rv;
+        }
     }
     else {
         /* Pipes on Win9* are blocking. Live with it. */