Skip to content

Commit 8efe8ea

Browse files
covenercovener
committed
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
1 parent b3d3ded commit 8efe8ea

File tree

6 files changed

+121
-24
lines changed

6 files changed

+121
-24
lines changed

‎docs/manual/mod/core.xml‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5201,8 +5201,12 @@ hostname or IP address</description>
52015201
</highlight>
52025202

52035203
<note type="warning"><title>Security</title>
5204-
<p>UNC paths accessed outside of request processing, such as during startup,
5205-
are not necessarily checked against the hosts configured with this directive.</p>
5204+
<p>The values specified by this directive are only checked by some
5205+
components of the server, prior to accessing filesystem paths that
5206+
may be inadvertently derived from untrusted inputs. </p>
5207+
<p> Windows systems should be isolated at the network layer from
5208+
making outbound SMB/NTLM calls to unexpected destinations as a
5209+
more comprehensive and pre-emptive measure.</p>
52065210
</note>
52075211

52085212
<note type="warning"><title>Directive Ordering</title>

‎include/ap_mmn.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,14 +610,15 @@
610610
* 20120211.138 (2.4.63-dev) Add is_host_matchable to proxy_worker_shared
611611
* 20120211.139 (2.4.63-dev) Add dav_get_base_path() to mod_dav
612612
* 20120211.140 (2.4.64-dev) Add ap_set_time_process_request() to scoreboard.h
613+
* 20120211.141 (2.4.64-dev) add ap_stat_check() to httpd.h
613614
*/
614615

615616
#define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
616617

617618
#ifndef MODULE_MAGIC_NUMBER_MAJOR
618619
#define MODULE_MAGIC_NUMBER_MAJOR 20120211
619620
#endif
620-
#define MODULE_MAGIC_NUMBER_MINOR 140 /* 0...n */
621+
#define MODULE_MAGIC_NUMBER_MINOR 141 /* 0...n */
621622

622623
/**
623624
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a

‎include/httpd.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2708,6 +2708,15 @@ AP_DECLARE(apr_status_t) ap_filepath_merge(char **newpath,
27082708
#define AP_SLASHES "/"
27092709
#endif
27102710

2711+
/**
2712+
* Validates the path parameter is safe to pass to stat-like calls.
2713+
* @param path The filesystem path to cehck
2714+
* @param p a pool for temporary allocations
2715+
* @return APR_SUCCESS if the stat-like call should be allowed
2716+
*/
2717+
AP_DECLARE(apr_status_t) ap_stat_check(const char *path, apr_pool_t *pool);
2718+
2719+
27112720
#ifdef __cplusplus
27122721
}
27132722
#endif

‎modules/mappers/mod_rewrite.c‎

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,12 @@ typedef enum {
319319
to be returned in r->status */
320320
} rule_return_type;
321321

322+
typedef enum {
323+
COND_RC_NOMATCH = 0, /* the cond didn't match */
324+
COND_RC_MATCH = 1, /* the cond matched */
325+
COND_RC_STATUS_SET = 3 /* The condition eval set a final r->status */
326+
} cond_return_type;
327+
322328
typedef struct {
323329
char *input; /* Input string of RewriteCond */
324330
char *pattern; /* the RegExp pattern string */
@@ -4103,62 +4109,87 @@ static APR_INLINE int compare_lexicography(char *a, char *b)
41034109
/*
41044110
* Apply a single rewriteCond
41054111
*/
4106-
static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
4112+
static cond_return_type apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
41074113
{
41084114
char *input = NULL;
41094115
apr_finfo_t sb;
41104116
request_rec *rsub, *r = ctx->r;
41114117
ap_regmatch_t regmatch[AP_MAX_REG_MATCH];
4112-
int rc = 0;
4118+
int rc = COND_RC_NOMATCH;
41134119
int basis;
41144120

41154121
if (p->ptype != CONDPAT_AP_EXPR)
41164122
input = do_expand(p->input, ctx, NULL, NULL);
41174123

41184124
switch (p->ptype) {
41194125
case CONDPAT_FILE_EXISTS:
4126+
if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
4127+
r->status = HTTP_FORBIDDEN;
4128+
rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
4129+
return COND_RC_STATUS_SET;
4130+
}
41204131
if ( apr_stat(&sb, input, APR_FINFO_MIN, r->pool) == APR_SUCCESS
41214132
&& sb.filetype == APR_REG) {
4122-
rc = 1;
4133+
rc = COND_RC_MATCH;
41234134
}
41244135
break;
41254136

41264137
case CONDPAT_FILE_SIZE:
4138+
if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
4139+
r->status = HTTP_FORBIDDEN;
4140+
rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
4141+
return COND_RC_STATUS_SET;
4142+
}
41274143
if ( apr_stat(&sb, input, APR_FINFO_MIN, r->pool) == APR_SUCCESS
41284144
&& sb.filetype == APR_REG && sb.size > 0) {
4129-
rc = 1;
4145+
rc = COND_RC_MATCH;
41304146
}
41314147
break;
41324148

41334149
case CONDPAT_FILE_LINK:
4150+
if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
4151+
r->status = HTTP_FORBIDDEN;
4152+
rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
4153+
return COND_RC_STATUS_SET;
4154+
}
41344155
#if !defined(OS2)
41354156
if ( apr_stat(&sb, input, APR_FINFO_MIN | APR_FINFO_LINK,
41364157
r->pool) == APR_SUCCESS
41374158
&& sb.filetype == APR_LNK) {
4138-
rc = 1;
4159+
rc = COND_RC_MATCH;
41394160
}
41404161
#endif
41414162
break;
41424163

41434164
case CONDPAT_FILE_DIR:
4165+
if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
4166+
r->status = HTTP_FORBIDDEN;
4167+
rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
4168+
return COND_RC_STATUS_SET;
4169+
}
41444170
if ( apr_stat(&sb, input, APR_FINFO_MIN, r->pool) == APR_SUCCESS
41454171
&& sb.filetype == APR_DIR) {
4146-
rc = 1;
4172+
rc = COND_RC_MATCH;
41474173
}
41484174
break;
41494175

41504176
case CONDPAT_FILE_XBIT:
4177+
if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
4178+
r->status = HTTP_FORBIDDEN;
4179+
rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
4180+
return COND_RC_STATUS_SET;
4181+
}
41514182
if ( apr_stat(&sb, input, APR_FINFO_PROT, r->pool) == APR_SUCCESS
41524183
&& (sb.protection & (APR_UEXECUTE | APR_GEXECUTE | APR_WEXECUTE))) {
4153-
rc = 1;
4184+
rc = COND_RC_MATCH;
41544185
}
41554186
break;
41564187

41574188
case CONDPAT_LU_URL:
41584189
if (*input && subreq_ok(r)) {
41594190
rsub = ap_sub_req_lookup_uri(input, r, NULL);
41604191
if (rsub->status < 400) {
4161-
rc = 1;
4192+
rc = COND_RC_MATCH;
41624193
}
41634194
rewritelog(r, 5, NULL, "RewriteCond URI (-U check: "
41644195
"path=%s -> status=%d", input, rsub->status);
@@ -4168,12 +4199,17 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
41684199

41694200
case CONDPAT_LU_FILE:
41704201
if (*input && subreq_ok(r)) {
4202+
if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
4203+
r->status = HTTP_FORBIDDEN;
4204+
rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
4205+
return COND_RC_STATUS_SET;
4206+
}
41714207
rsub = ap_sub_req_lookup_file(input, r, NULL);
41724208
if (rsub->status < 300 &&
41734209
/* double-check that file exists since default result is 200 */
41744210
apr_stat(&sb, rsub->filename, APR_FINFO_MIN,
41754211
r->pool) == APR_SUCCESS) {
4176-
rc = 1;
4212+
rc = COND_RC_MATCH;
41774213
}
41784214
rewritelog(r, 5, NULL, "RewriteCond file (-F check: path=%s "
41794215
"-> file=%s status=%d", input, rsub->filename,
@@ -4189,10 +4225,10 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
41894225
basis = 1;
41904226
test_str_g:
41914227
if (p->flags & CONDFLAG_NOCASE) {
4192-
rc = (strcasecmp(input, p->pattern) >= basis) ? 1 : 0;
4228+
rc = (strcasecmp(input, p->pattern) >= basis) ? COND_RC_MATCH : COND_RC_NOMATCH;
41934229
}
41944230
else {
4195-
rc = (compare_lexicography(input, p->pattern) >= basis) ? 1 : 0;
4231+
rc = (compare_lexicography(input, p->pattern) >= basis) ? COND_RC_MATCH : COND_RC_NOMATCH;
41964232
}
41974233
break;
41984234

@@ -4203,10 +4239,10 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
42034239
basis = -1;
42044240
test_str_l:
42054241
if (p->flags & CONDFLAG_NOCASE) {
4206-
rc = (strcasecmp(input, p->pattern) <= basis) ? 1 : 0;
4242+
rc = (strcasecmp(input, p->pattern) <= basis) ? COND_RC_MATCH : COND_RC_NOMATCH;
42074243
}
42084244
else {
4209-
rc = (compare_lexicography(input, p->pattern) <= basis) ? 1 : 0;
4245+
rc = (compare_lexicography(input, p->pattern) <= basis) ? COND_RC_MATCH : COND_RC_NOMATCH;
42104246
}
42114247
break;
42124248

@@ -4237,7 +4273,10 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
42374273
rewritelog(r, 1, ctx->perdir,
42384274
"RewriteCond: expr='%s' evaluation failed: %s",
42394275
p->pattern - p->pskip, err);
4240-
rc = 0;
4276+
rc = COND_RC_NOMATCH;
4277+
}
4278+
else {
4279+
rc = COND_RC_MATCH;
42414280
}
42424281
/* update briRC backref info */
42434282
if (rc && !(p->flags & CONDFLAG_NOTMATCH)) {
@@ -4258,7 +4297,7 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx)
42584297
break;
42594298
}
42604299

4261-
if (p->flags & CONDFLAG_NOTMATCH) {
4300+
if (p->flags & CONDFLAG_NOTMATCH && rc <= COND_RC_MATCH) {
42624301
rc = !rc;
42634302
}
42644303

@@ -4391,6 +4430,12 @@ static rule_return_type apply_rewrite_rule(rewriterule_entry *p,
43914430
rewritecond_entry *c = &conds[i];
43924431

43934432
rc = apply_rewrite_cond(c, ctx);
4433+
4434+
/* Error while evaluating cond, r->status set */
4435+
if (COND_RC_STATUS_SET == rc) {
4436+
return RULE_RC_STATUS_SET;
4437+
}
4438+
43944439
/*
43954440
* Reset vary_this if the novary flag is set for this condition.
43964441
*/

‎server/core.c‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5515,9 +5515,13 @@ static apr_status_t core_insert_network_bucket(conn_rec *c,
55155515
}
55165516

55175517
static apr_status_t core_dirwalk_stat(apr_finfo_t *finfo, request_rec *r,
5518-
apr_int32_t wanted)
5518+
apr_int32_t wanted)
55195519
{
5520-
return apr_stat(finfo, r->filename, wanted, r->pool);
5520+
apr_status_t rv = ap_stat_check(r->filename, r->pool);
5521+
if (rv == APR_SUCCESS) {
5522+
rv = apr_stat(finfo, r->filename, wanted, r->pool);
5523+
}
5524+
return rv;
55215525
}
55225526

55235527
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)
56805684
*s++ = '/';
56815685

56825686
ap_log_error(APLOG_MARK, APLOG_TRACE4, 0, ap_server_conf,
5683-
"ap_filepath_merge: check converted path %s allowed %d",
5687+
"check_unc: check converted path %s allowed %d",
56845688
teststring,
56855689
sconf->unc_list ? sconf->unc_list->nelts : 0);
56865690

@@ -5692,19 +5696,19 @@ static apr_status_t check_unc(const char *path, apr_pool_t *p)
56925696
!ap_cstr_casecmp(uri.hostinfo, configured_unc))) {
56935697
rv = APR_SUCCESS;
56945698
ap_log_error(APLOG_MARK, APLOG_TRACE4, 0, ap_server_conf,
5695-
"ap_filepath_merge: match %s %s",
5699+
"check_unc: match %s %s",
56965700
uri.hostinfo, configured_unc);
56975701
break;
56985702
}
56995703
else {
57005704
ap_log_error(APLOG_MARK, APLOG_TRACE4, 0, ap_server_conf,
5701-
"ap_filepath_merge: no match %s %s", uri.hostinfo,
5705+
"check_unc: no match %s %s", uri.hostinfo,
57025706
configured_unc);
57035707
}
57045708
}
57055709
if (rv != APR_SUCCESS) {
57065710
ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf, APLOGNO(10504)
5707-
"ap_filepath_merge: UNC path %s not allowed by UNCList", teststring);
5711+
"check_unc: UNC path %s not allowed by UNCList", teststring);
57085712
}
57095713

57105714
return rv;
@@ -5734,6 +5738,17 @@ AP_DECLARE(apr_status_t) ap_filepath_merge(char **newpath,
57345738
#endif
57355739
}
57365740

5741+
#ifdef WIN32
5742+
AP_DECLARE(apr_status_t) ap_stat_check(const char *path, apr_pool_t *p)
5743+
{
5744+
return check_unc(path, p);
5745+
}
5746+
#else
5747+
AP_DECLARE(apr_status_t) ap_stat_check(const char *path, apr_pool_t *p)
5748+
{
5749+
return APR_SUCCESS;
5750+
}
5751+
#endif
57375752

57385753
static void register_hooks(apr_pool_t *p)
57395754
{

‎server/util_expr_eval.c‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,10 +1147,21 @@ static const char *file_func(ap_expr_eval_ctx_t *ctx, const void *data,
11471147
return buf;
11481148
}
11491149

1150+
static apr_status_t stat_check(ap_expr_eval_ctx_t *ctx, const void *data, const char *arg)
1151+
{
1152+
apr_status_t rv = APR_SUCCESS;
1153+
if (APR_SUCCESS != (rv = ap_stat_check(arg, ctx->p))) {
1154+
*ctx->err = apr_psprintf(ctx->p, "stat of %s not allowed", arg);
1155+
}
1156+
return rv;
1157+
}
11501158
static const char *filesize_func(ap_expr_eval_ctx_t *ctx, const void *data,
11511159
char *arg)
11521160
{
11531161
apr_finfo_t sb;
1162+
if (APR_SUCCESS != stat_check(ctx, data, arg)) {
1163+
return "";
1164+
}
11541165
if (apr_stat(&sb, arg, APR_FINFO_MIN, ctx->p) == APR_SUCCESS
11551166
&& sb.filetype == APR_REG && sb.size > 0)
11561167
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
11851196
{
11861197
apr_finfo_t sb;
11871198
const char *name = (const char *)data;
1199+
if (APR_SUCCESS != stat_check(ctx, data, arg)) {
1200+
return FALSE;
1201+
}
11881202
if (apr_stat(&sb, arg, APR_FINFO_MIN, ctx->p) != APR_SUCCESS)
11891203
return FALSE;
11901204
switch (name[0]) {
@@ -1206,6 +1220,9 @@ static int op_file_link(ap_expr_eval_ctx_t *ctx, const void *data, const char *a
12061220
{
12071221
#if !defined(OS2)
12081222
apr_finfo_t sb;
1223+
if (APR_SUCCESS != stat_check(ctx, data, arg)) {
1224+
return FALSE;
1225+
}
12091226
if (apr_stat(&sb, arg, APR_FINFO_MIN | APR_FINFO_LINK, ctx->p) == APR_SUCCESS
12101227
&& sb.filetype == APR_LNK) {
12111228
return TRUE;
@@ -1217,6 +1234,9 @@ static int op_file_link(ap_expr_eval_ctx_t *ctx, const void *data, const char *a
12171234
static int op_file_xbit(ap_expr_eval_ctx_t *ctx, const void *data, const char *arg)
12181235
{
12191236
apr_finfo_t sb;
1237+
if (APR_SUCCESS != stat_check(ctx, data, arg)) {
1238+
return FALSE;
1239+
}
12201240
if (apr_stat(&sb, arg, APR_FINFO_PROT| APR_FINFO_LINK, ctx->p) == APR_SUCCESS
12211241
&& (sb.protection & (APR_UEXECUTE | APR_GEXECUTE | APR_WEXECUTE))) {
12221242
return TRUE;
@@ -1253,6 +1273,9 @@ static int op_file_subr(ap_expr_eval_ctx_t *ctx, const void *data, const char *a
12531273
request_rec *rsub, *r = ctx->r;
12541274
if (!r)
12551275
return FALSE;
1276+
if (APR_SUCCESS != stat_check(ctx, data, arg)) {
1277+
return FALSE;
1278+
}
12561279
rsub = ap_sub_req_lookup_file(arg, r, NULL);
12571280
if (rsub->status < 300 &&
12581281
/* double-check that file exists since default result is 200 */

0 commit comments

Comments
 (0)