From: Eric Covener Date: Mon, 7 Jul 2025 12:02:22 +0000 (+0000) Subject: backport 1927033 from trunk X-Git-Tag: 2.4.64-rc1-candidate~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8efe8ea18c6f7123c5779bb4d9551ccf407dc0c4;p=thirdparty%2Fapache%2Fhttpd.git 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 8fe924172c0..7c6bd23857d 100644 --- a/docs/manual/mod/core.xml +++ b/docs/manual/mod/core.xml @@ -5201,8 +5201,12 @@ hostname or IP address 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 fce2dc9330c..8164fe06511 100644 --- 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 6c7484fc040..b4305b09f2f 100644 --- 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 93430e5952e..f9fcd61400f 100644 --- 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 cfd3c7915e6..fbda9b6dbe0 100644 --- 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 db4be957d80..038d5a06d78 100644 --- 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 */