expand UNC checking

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1927033 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/docs/manual/mod/core.xml b/docs/manual/mod/core.xml
index 7a0a6565377e755719c667f2089c6b5a8bd03c31..7d53270224508426501a414a73f184736af80755 100644 (file)
--- a/docs/manual/mod/core.xml
+++ b/docs/manual/mod/core.xml
@@ -5351,8 +5351,12 @@ recognized methods to modules.</p>
     </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 c9a0d2642ac3cf4182724db6d7dfb3cde68abf58..09770fa1a574b60dbbfd6e22c4f7be2d1ce3f1ed 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -735,6 +735,7 @@
  * 20211221.27 (2.5.1-dev) Add sock_proto to proxy_worker_shared, and AP_LISTEN_MPTCP
  * 20211221.28 (2.5.1-dev) Add dav_get_base_path() to mod_dav
  * 20211221.29 (2.5.1-dev) Add ap_set_time_process_request() to scoreboard.h
+ * 20211221.30 (2.5.1-dev) Add ap_stat_check() to httpd.h
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
@@ -742,7 +743,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20211221
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 29             /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 30             /* 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 9a9d922c40c0928fdca28c11587f9cf72f1d49c3..577891e567a9d9ba5a61ea60f03db054f6bedbfa 100644 (file)
--- a/include/httpd.h
+++ b/include/httpd.h
@@ -2890,6 +2890,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 b565fb6974036cd848c4c5f3469792ac42d1d27d..a6c35a0be1f82bb6de861bc3816bb473ce9b8124 100644 (file)
--- a/modules/mappers/mod_rewrite.c
+++ b/modules/mappers/mod_rewrite.c
@@ -310,6 +310,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     */
@@ -4111,13 +4117,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, apr_pool_t *pool)
+static cond_return_type apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx, apr_pool_t *pool)
 {
     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)
@@ -4125,40 +4131,65 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx, apr_pool_t
 
     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;
 
@@ -4166,7 +4197,7 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx, apr_pool_t
         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);
@@ -4176,12 +4207,17 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx, apr_pool_t
 
     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,
@@ -4197,10 +4233,10 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx, apr_pool_t
         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;
 
@@ -4211,10 +4247,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;
 
@@ -4245,7 +4281,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)) {
@@ -4266,7 +4305,7 @@ test_str_l:
         break;
     }
 
-    if (p->flags & CONDFLAG_NOTMATCH) {
+    if (p->flags & CONDFLAG_NOTMATCH && rc <= COND_RC_MATCH) {
         rc = !rc;
     }
 
@@ -4399,6 +4438,12 @@ static rule_return_type apply_rewrite_rule(rewriterule_entry *p,
         rewritecond_entry *c = &conds[i];
 
         rc = apply_rewrite_cond(c, ctx,  ctx->temp_pool ? ctx->temp_pool : r->pool);
+
+        /* 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 96b0841a78e26e67c863f357c1aa972c09350c6b..f17113b2b8d7fce466e744109852c862a4359ab2 100644 (file)
--- a/server/core.c
+++ b/server/core.c
@@ -5884,9 +5884,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)
@@ -6047,7 +6051,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);
 
@@ -6059,19 +6063,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;
@@ -6101,6 +6105,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 7a2a7da01540d41fd58a1855946e01bc3c45fec2..f9db28e70442cf4c86feabc404a4e16375374ed4 100644 (file)
--- a/server/util_expr_eval.c
+++ b/server/util_expr_eval.c
@@ -1527,10 +1527,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);
@@ -1542,6 +1553,9 @@ static const char *filemod_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.mtime > 0)
         return apr_psprintf(ctx->p, "%" APR_OFF_T_FMT, (apr_off_t)sb.mtime);
@@ -1577,6 +1591,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]) {
@@ -1598,6 +1615,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;
@@ -1609,6 +1629,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;
@@ -1645,6 +1668,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 */