backport 1927033 from trunk

  expand UNC checking

Reviewed By: covener, jfclere, rpluem

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

diff --git a/docs/manual/mod/core.xml b/docs/manual/mod/core.xml
index 8fe924172c0850677396c59498e6cef4c56ece25..7c6bd23857d3a56c87befde8fa8db85584abb27a 100644 (file)
--- a/docs/manual/mod/core.xml
+++ b/docs/manual/mod/core.xml
@@ -5201,8 +5201,12 @@ hostname or IP address</description>
     </highlight>
 
     <note type="warning"><title>Security</title>
-    <p>UNC paths accessed outside of request processing, such as during startup,
-    are not necessarily checked against the hosts configured with this directive.</p>
+        <p>The values specified by this directive are only checked by some
+        components of the server, prior to accessing filesystem paths that
+        may be inadvertently derived from untrusted inputs. </p>
+        <p> Windows systems should be isolated at the network layer from
+        making outbound SMB/NTLM calls to unexpected destinations as a
+        more comprehensive and pre-emptive measure.</p>
     </note>
 
     <note type="warning"><title>Directive Ordering</title>

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index fce2dc9330ccd9bf0d3d8ad8caa4ff7d8b7b9b57..8164fe06511e348cf68fbf0e38b6490112cf07da 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -610,6 +610,7 @@
  * 20120211.138 (2.4.63-dev) Add is_host_matchable to proxy_worker_shared
  * 20120211.139 (2.4.63-dev) Add dav_get_base_path() to mod_dav
  * 20120211.140 (2.4.64-dev) Add ap_set_time_process_request() to scoreboard.h
+ * 20120211.141 (2.4.64-dev) add ap_stat_check() to httpd.h
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
@@ -617,7 +618,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20120211
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 140                 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 141                 /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

diff --git a/include/httpd.h b/include/httpd.h
index 6c7484fc040b4420b4707b0240466b635615b517..b4305b09f2fecb9d568a637f9c2d42037d08bc4f 100644 (file)
--- a/include/httpd.h
+++ b/include/httpd.h
@@ -2708,6 +2708,15 @@ AP_DECLARE(apr_status_t) ap_filepath_merge(char **newpath,
 #define AP_SLASHES "/"
 #endif
 
+/**
+ * Validates the path parameter is safe to pass to stat-like calls.
+ * @param path The filesystem path to cehck
+ * @param p a pool for temporary allocations
+ * @return APR_SUCCESS if the stat-like call should be allowed
+ */
+AP_DECLARE(apr_status_t) ap_stat_check(const char *path, apr_pool_t *pool);
+
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/modules/mappers/mod_rewrite.c b/modules/mappers/mod_rewrite.c
index 93430e5952e0d9d1e973f542221482c2fff11a70..f9fcd61400fdb8b425e2207558b2c9470849f006 100644 (file)
--- a/modules/mappers/mod_rewrite.c
+++ b/modules/mappers/mod_rewrite.c
@@ -319,6 +319,12 @@ typedef enum {
                                to be returned in r->status */
 } rule_return_type;
 
+typedef enum {
+  COND_RC_NOMATCH = 0,      /* the cond didn't match                        */
+  COND_RC_MATCH = 1,        /* the cond matched                             */
+  COND_RC_STATUS_SET = 3    /* The condition eval set a final r->status     */
+} cond_return_type;
+
 typedef struct {
     char           *input;   /* Input string of RewriteCond   */
     char           *pattern; /* the RegExp pattern string     */
@@ -4103,13 +4109,13 @@ static APR_INLINE int compare_lexicography(char *a, char *b)
 /*
  * Apply a single rewriteCond
  */
-static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
+static cond_return_type apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
 {
     char *input = NULL;
     apr_finfo_t sb;
     request_rec *rsub, *r = ctx->r;
     ap_regmatch_t regmatch[AP_MAX_REG_MATCH];
-    int rc = 0;
+    int rc = COND_RC_NOMATCH;
     int basis;
 
     if (p->ptype != CONDPAT_AP_EXPR)
@@ -4117,40 +4123,65 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
 
     switch (p->ptype) {
     case CONDPAT_FILE_EXISTS:
+        if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
+            r->status = HTTP_FORBIDDEN;
+            rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
+            return COND_RC_STATUS_SET;
+        }
         if (   apr_stat(&sb, input, APR_FINFO_MIN, r->pool) == APR_SUCCESS
             && sb.filetype == APR_REG) {
-            rc = 1;
+            rc = COND_RC_MATCH;
         }
         break;
 
     case CONDPAT_FILE_SIZE:
+        if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
+            r->status = HTTP_FORBIDDEN;
+            rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
+            return COND_RC_STATUS_SET;
+        }
         if (   apr_stat(&sb, input, APR_FINFO_MIN, r->pool) == APR_SUCCESS
             && sb.filetype == APR_REG && sb.size > 0) {
-            rc = 1;
+            rc = COND_RC_MATCH;
         }
         break;
 
     case CONDPAT_FILE_LINK:
+        if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
+            r->status = HTTP_FORBIDDEN;
+            rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
+            return COND_RC_STATUS_SET;
+        }
 #if !defined(OS2)
         if (   apr_stat(&sb, input, APR_FINFO_MIN | APR_FINFO_LINK,
                         r->pool) == APR_SUCCESS
             && sb.filetype == APR_LNK) {
-            rc = 1;
+            rc = COND_RC_MATCH;
         }
 #endif
         break;
 
     case CONDPAT_FILE_DIR:
+        if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
+            r->status = HTTP_FORBIDDEN;
+            rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
+            return COND_RC_STATUS_SET;
+        }
         if (   apr_stat(&sb, input, APR_FINFO_MIN, r->pool) == APR_SUCCESS
             && sb.filetype == APR_DIR) {
-            rc = 1;
+            rc = COND_RC_MATCH;
         }
         break;
 
     case CONDPAT_FILE_XBIT:
+        if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
+            r->status = HTTP_FORBIDDEN;
+            rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
+            return COND_RC_STATUS_SET;
+        }
         if (   apr_stat(&sb, input, APR_FINFO_PROT, r->pool) == APR_SUCCESS
             && (sb.protection & (APR_UEXECUTE | APR_GEXECUTE | APR_WEXECUTE))) {
-            rc = 1;
+            rc = COND_RC_MATCH;
         }
         break;
 
@@ -4158,7 +4189,7 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
         if (*input && subreq_ok(r)) {
             rsub = ap_sub_req_lookup_uri(input, r, NULL);
             if (rsub->status < 400) {
-                rc = 1;
+                rc = COND_RC_MATCH;
             }
             rewritelog(r, 5, NULL, "RewriteCond URI (-U check: "
                         "path=%s -> status=%d", input, rsub->status);
@@ -4168,12 +4199,17 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
 
     case CONDPAT_LU_FILE:
         if (*input && subreq_ok(r)) {
+            if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
+                r->status = HTTP_FORBIDDEN;
+                rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
+                return COND_RC_STATUS_SET;
+            }
             rsub = ap_sub_req_lookup_file(input, r, NULL);
             if (rsub->status < 300 &&
                 /* double-check that file exists since default result is 200 */
                 apr_stat(&sb, rsub->filename, APR_FINFO_MIN,
                          r->pool) == APR_SUCCESS) {
-                rc = 1;
+                rc = COND_RC_MATCH;
             }
             rewritelog(r, 5, NULL, "RewriteCond file (-F check: path=%s "
                         "-> file=%s status=%d", input, rsub->filename,
@@ -4189,10 +4225,10 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
         basis = 1;
 test_str_g:
         if (p->flags & CONDFLAG_NOCASE) {
-            rc = (strcasecmp(input, p->pattern) >= basis) ? 1 : 0;
+            rc = (strcasecmp(input, p->pattern) >= basis) ? COND_RC_MATCH : COND_RC_NOMATCH;
         }
         else {
-            rc = (compare_lexicography(input, p->pattern) >= basis) ? 1 : 0;
+            rc = (compare_lexicography(input, p->pattern) >= basis) ? COND_RC_MATCH : COND_RC_NOMATCH;
         }
         break;
 
@@ -4203,10 +4239,10 @@ test_str_g:
         basis = -1;
 test_str_l:
         if (p->flags & CONDFLAG_NOCASE) {
-            rc = (strcasecmp(input, p->pattern) <= basis) ? 1 : 0;
+            rc = (strcasecmp(input, p->pattern) <= basis) ? COND_RC_MATCH : COND_RC_NOMATCH;
         }
         else {
-            rc = (compare_lexicography(input, p->pattern) <= basis) ? 1 : 0;
+            rc = (compare_lexicography(input, p->pattern) <= basis) ? COND_RC_MATCH : COND_RC_NOMATCH;
         }
         break;
 
@@ -4237,7 +4273,10 @@ test_str_l:
                 rewritelog(r, 1, ctx->perdir,
                             "RewriteCond: expr='%s' evaluation failed: %s",
                             p->pattern - p->pskip, err);
-                rc = 0;
+                rc = COND_RC_NOMATCH;
+            }
+            else {
+                rc = COND_RC_MATCH;
             }
             /* update briRC backref info */
             if (rc && !(p->flags & CONDFLAG_NOTMATCH)) {
@@ -4258,7 +4297,7 @@ test_str_l:
         break;
     }
 
-    if (p->flags & CONDFLAG_NOTMATCH) {
+    if (p->flags & CONDFLAG_NOTMATCH && rc <= COND_RC_MATCH) {
         rc = !rc;
     }
 
@@ -4391,6 +4430,12 @@ static rule_return_type apply_rewrite_rule(rewriterule_entry *p,
         rewritecond_entry *c = &conds[i];
 
         rc = apply_rewrite_cond(c, ctx);
+
+        /* Error while evaluating cond, r->status set */
+        if (COND_RC_STATUS_SET == rc) {
+            return RULE_RC_STATUS_SET;
+        }
+
         /*
          * Reset vary_this if the novary flag is set for this condition.
          */

diff --git a/server/core.c b/server/core.c
index cfd3c7915e6ed5969fef07a023364bd85d26aacb..fbda9b6dbe01772a25d18d0a7c5d0c4ee680c2f4 100644 (file)
--- a/server/core.c
+++ b/server/core.c
@@ -5515,9 +5515,13 @@ static apr_status_t core_insert_network_bucket(conn_rec *c,
 }
 
 static apr_status_t core_dirwalk_stat(apr_finfo_t *finfo, request_rec *r,
-                                      apr_int32_t wanted) 
+                                      apr_int32_t wanted)
 {
-    return apr_stat(finfo, r->filename, wanted, r->pool);
+    apr_status_t rv = ap_stat_check(r->filename, r->pool);
+    if (rv == APR_SUCCESS) {
+        rv = apr_stat(finfo, r->filename, wanted, r->pool);
+    }
+    return rv;
 }
 
 static void core_dump_config(apr_pool_t *p, server_rec *s)
@@ -5680,7 +5684,7 @@ static apr_status_t check_unc(const char *path, apr_pool_t *p)
     *s++ = '/';
 
     ap_log_error(APLOG_MARK, APLOG_TRACE4, 0, ap_server_conf, 
-                 "ap_filepath_merge: check converted path %s allowed %d", 
+                 "check_unc: check converted path %s allowed %d", 
                  teststring,
                  sconf->unc_list ? sconf->unc_list->nelts : 0);
 
@@ -5692,19 +5696,19 @@ static apr_status_t check_unc(const char *path, apr_pool_t *p)
                  !ap_cstr_casecmp(uri.hostinfo, configured_unc))) { 
             rv = APR_SUCCESS;
             ap_log_error(APLOG_MARK, APLOG_TRACE4, 0, ap_server_conf, 
-                         "ap_filepath_merge: match %s %s", 
+                         "check_unc: match %s %s", 
                          uri.hostinfo, configured_unc);
             break;
         }
         else { 
             ap_log_error(APLOG_MARK, APLOG_TRACE4, 0, ap_server_conf, 
-                         "ap_filepath_merge: no match %s %s", uri.hostinfo, 
+                         "check_unc: no match %s %s", uri.hostinfo, 
                          configured_unc);
         }
     }
     if (rv != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf, APLOGNO(10504)
-                     "ap_filepath_merge: UNC path %s not allowed by UNCList", teststring);
+                     "check_unc: UNC path %s not allowed by UNCList", teststring);
     }
 
     return rv;
@@ -5734,6 +5738,17 @@ AP_DECLARE(apr_status_t) ap_filepath_merge(char **newpath,
 #endif
 }
 
+#ifdef WIN32
+AP_DECLARE(apr_status_t) ap_stat_check(const char *path, apr_pool_t *p)
+{
+   return check_unc(path, p);
+}
+#else
+AP_DECLARE(apr_status_t) ap_stat_check(const char *path, apr_pool_t *p)
+{
+    return APR_SUCCESS;
+}
+#endif
 
 static void register_hooks(apr_pool_t *p)
 {

diff --git a/server/util_expr_eval.c b/server/util_expr_eval.c
index db4be957d80eebab9a4abb2a2a330082a5283529..038d5a06d7896e4dbd58a8e442e4d96db5116dd7 100644 (file)
--- a/server/util_expr_eval.c
+++ b/server/util_expr_eval.c
@@ -1147,10 +1147,21 @@ static const char *file_func(ap_expr_eval_ctx_t *ctx, const void *data,
     return buf;
 }
 
+static apr_status_t stat_check(ap_expr_eval_ctx_t *ctx, const void *data, const char *arg)
+{
+    apr_status_t rv = APR_SUCCESS;
+    if (APR_SUCCESS != (rv = ap_stat_check(arg, ctx->p))) {
+        *ctx->err = apr_psprintf(ctx->p, "stat of %s not allowed", arg);
+    }
+    return rv;
+}
 static const char *filesize_func(ap_expr_eval_ctx_t *ctx, const void *data,
                                   char *arg)
 {
     apr_finfo_t sb;
+    if (APR_SUCCESS != stat_check(ctx, data, arg)) {
+        return "";
+    }
     if (apr_stat(&sb, arg, APR_FINFO_MIN, ctx->p) == APR_SUCCESS
         && sb.filetype == APR_REG && sb.size > 0)
         return apr_psprintf(ctx->p, "%" APR_OFF_T_FMT, sb.size);
@@ -1185,6 +1196,9 @@ static int op_file_min(ap_expr_eval_ctx_t *ctx, const void *data, const char *ar
 {
     apr_finfo_t sb;
     const char *name = (const char *)data;
+    if (APR_SUCCESS != stat_check(ctx, data, arg)) {
+        return FALSE;
+    }
     if (apr_stat(&sb, arg, APR_FINFO_MIN, ctx->p) != APR_SUCCESS)
         return FALSE;
     switch (name[0]) {
@@ -1206,6 +1220,9 @@ static int op_file_link(ap_expr_eval_ctx_t *ctx, const void *data, const char *a
 {
 #if !defined(OS2)
     apr_finfo_t sb;
+    if (APR_SUCCESS != stat_check(ctx, data, arg)) {
+        return FALSE;
+    }
     if (apr_stat(&sb, arg, APR_FINFO_MIN | APR_FINFO_LINK, ctx->p) == APR_SUCCESS
         && sb.filetype == APR_LNK) {
         return TRUE;
@@ -1217,6 +1234,9 @@ static int op_file_link(ap_expr_eval_ctx_t *ctx, const void *data, const char *a
 static int op_file_xbit(ap_expr_eval_ctx_t *ctx, const void *data, const char *arg)
 {
     apr_finfo_t sb;
+    if (APR_SUCCESS != stat_check(ctx, data, arg)) {
+        return FALSE;
+    }
     if (apr_stat(&sb, arg, APR_FINFO_PROT| APR_FINFO_LINK, ctx->p) == APR_SUCCESS
         && (sb.protection & (APR_UEXECUTE | APR_GEXECUTE | APR_WEXECUTE))) {
         return TRUE;
@@ -1253,6 +1273,9 @@ static int op_file_subr(ap_expr_eval_ctx_t *ctx, const void *data, const char *a
     request_rec *rsub, *r = ctx->r;
     if (!r)
         return FALSE;
+    if (APR_SUCCESS != stat_check(ctx, data, arg)) {
+        return FALSE;
+    }
     rsub = ap_sub_req_lookup_file(arg, r, NULL);
     if (rsub->status < 300 &&
         /* double-check that file exists since default result is 200 */