From dd98030cb399e962aa6058c35893491a9ef7d147 Mon Sep 17 00:00:00 2001 From: Eric Covener Date: Mon, 7 Jul 2025 11:49:48 +0000 Subject: [PATCH] expand UNC checking git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1927033 13f79535-47bb-0310-9956-ffa450edef68 --- docs/manual/mod/core.xml | 8 +++- include/ap_mmn.h | 3 +- include/httpd.h | 9 +++++ modules/mappers/mod_rewrite.c | 75 ++++++++++++++++++++++++++++------- server/core.c | 27 ++++++++++--- server/util_expr_eval.c | 26 ++++++++++++ 6 files changed, 124 insertions(+), 24 deletions(-) diff --git a/docs/manual/mod/core.xml b/docs/manual/mod/core.xml index 7a0a656537..7d53270224 100644 --- a/docs/manual/mod/core.xml +++ b/docs/manual/mod/core.xml @@ -5351,8 +5351,12 @@ recognized methods to modules.

Security -

UNC paths accessed outside of request processing, such as during startup, - are not necessarily checked against the hosts configured with this directive.

+

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.

+

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.

 Directive Ordering diff --git a/include/ap_mmn.h b/include/ap_mmn.h index c9a0d2642a..09770fa1a5 100644 --- 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 9a9d922c40..577891e567 100644 --- 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 b565fb6974..a6c35a0be1 100644 --- 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 96b0841a78..f17113b2b8 100644 --- 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 7a2a7da015..f9db28e704 100644 --- 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 */