From: Jeff Trawick <trawick@apache.org>
Date: Thu, 13 May 2010 16:06:25 +0000 (+0000)
Subject: merge r814844 from 2.2.x branch (trunk revs 814652 and 814785):
X-Git-Tag: 2.0.64~48
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=65ee268d15a35a3324ba899e4a73f586c6d8640c;p=thirdparty%2Fapache%2Fhttpd.git

merge r814844 from 2.2.x branch (trunk revs 814652 and 814785):

  *) SECURITY: CVE-2009-3094 (cve.mitre.org)
     mod_proxy_ftp: NULL pointer dereference on error paths.
     [Stefan Fritsch <sf fritsch.de>, Joe Orton]

Reviewed by: pgollucci, poirier, trawick


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.0.x@943925 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/CHANGES b/CHANGES
index 599d594a2af..6cc68bbefd9 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.0.64
 
+  *) SECURITY: CVE-2009-3094 (cve.mitre.org)
+     mod_proxy_ftp: NULL pointer dereference on error paths.
+     [Stefan Fritsch <sf fritsch.de>, Joe Orton]
+
   *) SECURITY: CVE-2009-3555 (cve.mitre.org)
      mod_ssl: A partial fix for the TLS renegotiation prefix injection attack
      for OpenSSL versions prior to 0.9.8l; reject any client-initiated
diff --git a/STATUS b/STATUS
index 93c0b65f70a..f8e91433034 100644
--- a/STATUS
+++ b/STATUS
@@ -141,13 +141,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
     +1: pgollucci, poirier, rjung
       PG: whomever proposed this should vote for it
 
-  * mod_proxy_ftp, CVE-2009-3094, NULL pointer dereference on error paths
-    Patch in 2.2.x branch:
-      http://svn.apache.org/viewvc?view=revision&revision=814844
-    Backport:
-      http://people.apache.org/~trawick/CVE-2009-3094-2.0.txt
-    +1: pgollucci, poirier, trawick
-
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ please place SVN revisions from trunk here, so it is easy to
     identify exactly what the proposed changes are!  Add all new
diff --git a/modules/proxy/proxy_ftp.c b/modules/proxy/proxy_ftp.c
index 3053c44748b..16abbdd226e 100644
--- a/modules/proxy/proxy_ftp.c
+++ b/modules/proxy/proxy_ftp.c
@@ -588,6 +588,31 @@ apr_status_t ap_proxy_send_dir_filter(ap_filter_t *f, apr_bucket_brigade *in)
     return APR_SUCCESS;
 }
 
+/* Parse EPSV reply and return port, or zero on error. */
+static apr_port_t parse_epsv_reply(const char *reply)
+{
+    const char *p;
+    char *ep;
+    long port;
+
+    /* Reply syntax per RFC 2428: "229 blah blah (|||port|)" where '|'
+     * can be any character in ASCII from 33-126, obscurely.  Verify
+     * the syntax. */
+    p = ap_strchr_c(reply, '(');
+    if (p == NULL || !p[1] || p[1] != p[2] || p[1] != p[3]
+        || p[4] == p[1]) {
+        return 0;
+    }
+
+    errno = 0;
+    port = strtol(p + 4, &ep, 10);
+    if (errno || port < 1 || port > 65535 || ep[0] != p[1] || ep[1] != ')') {
+        return 0;
+    }
+
+    return (apr_port_t)port;
+}
+
 /*
  * Generic "send FTP command to server" routine, using the control socket.
  * Returns the FTP returncode (3 digit code)
@@ -1232,26 +1257,11 @@ int ap_proxy_ftp_handler(request_rec *r, proxy_server_conf *conf,
             return ap_proxyerror(r, HTTP_BAD_GATEWAY, ftpmessage);
         }
         else if (rc == 229) {
-            char *pstr;
-            char *tok_cntx;
+            /* Parse the port out of the EPSV reply. */
+            data_port = parse_epsv_reply(ftpmessage);
 
-            pstr = ftpmessage;
-            pstr = apr_strtok(pstr, " ", &tok_cntx);    /* separate result code */
-            if (pstr != NULL) {
-                if (*(pstr + strlen(pstr) + 1) == '=') {
-                    pstr += strlen(pstr) + 2;
-                }
-                else {
-                    pstr = apr_strtok(NULL, "(", &tok_cntx);    /* separate address &
-                                                                 * port params */
-                    if (pstr != NULL)
-                        pstr = apr_strtok(NULL, ")", &tok_cntx);
-                }
-            }
-
-            if (pstr) {
+            if (data_port) {
                 apr_sockaddr_t *epsv_addr;
-                data_port = atoi(pstr + 3);
 
                 ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
                        "proxy: FTP: EPSV contacting remote host on port %d",
@@ -1287,10 +1297,6 @@ int ap_proxy_ftp_handler(request_rec *r, proxy_server_conf *conf,
                     connect = 1;
                 }
             }
-            else {
-                /* and try the regular way */
-                apr_socket_close(data_sock);
-            }
         }
     }
 
@@ -1372,10 +1378,6 @@ int ap_proxy_ftp_handler(request_rec *r, proxy_server_conf *conf,
                     connect = 1;
                 }
             }
-            else {
-                /* and try the regular way */
-                apr_socket_close(data_sock);
-            }
         }
     }
 /*bypass:*/
@@ -1840,7 +1842,9 @@ int ap_proxy_ftp_handler(request_rec *r, proxy_server_conf *conf,
                  * for a slow client to eat these bytes
                  */
                 ap_flush_conn(data);
-                apr_socket_close(data_sock);
+                if (data_sock) {
+                    apr_socket_close(data_sock);
+                }
                 data_sock = NULL;
                 ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
                              "proxy: FTP: data connection closed");