Skip to content

Commit dd98030

Browse files
covenercovener
committed
expand UNC checking
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1927033 13f79535-47bb-0310-9956-ffa450edef68
1 parent 32cf503 commit dd98030

File tree

6 files changed

+124
-24
lines changed

6 files changed

+124
-24
lines changed

‎docs/manual/mod/core.xml‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5351,8 +5351,12 @@ recognized methods to modules.</p>
53515351
</highlight>
53525352

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

53585362
<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
@@ -735,14 +735,15 @@
735735
* 20211221.27 (2.5.1-dev) Add sock_proto to proxy_worker_shared, and AP_LISTEN_MPTCP
736736
* 20211221.28 (2.5.1-dev) Add dav_get_base_path() to mod_dav
737737
* 20211221.29 (2.5.1-dev) Add ap_set_time_process_request() to scoreboard.h
738+
* 20211221.30 (2.5.1-dev) Add ap_stat_check() to httpd.h
738739
*/
739740

740741
#define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
741742

742743
#ifndef MODULE_MAGIC_NUMBER_MAJOR
743744
#define MODULE_MAGIC_NUMBER_MAJOR 20211221
744745
#endif
745-
#define MODULE_MAGIC_NUMBER_MINOR 29 /* 0...n */
746+
#define MODULE_MAGIC_NUMBER_MINOR 30 /* 0...n */
746747

747748
/**
748749
* 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
@@ -2890,6 +2890,15 @@ AP_DECLARE(apr_status_t) ap_filepath_merge(char **newpath,
28902890
#define AP_SLASHES "/"
28912891
#endif
28922892

2893+
/**
2894+
* Validates the path parameter is safe to pass to stat-like calls.
2895+
* @param path The filesystem path to cehck
2896+
* @param p a pool for temporary allocations
2897+
* @return APR_SUCCESS if the stat-like call should be allowed
2898+
*/
2899+
AP_DECLARE(apr_status_t) ap_stat_check(const char *path, apr_pool_t *pool);
2900+
2901+
28932902
#ifdef __cplusplus
28942903
}
28952904
#endif

‎modules/mappers/mod_rewrite.c‎

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

313+
typedef enum {
314+
COND_RC_NOMATCH = 0, /* the cond didn't match */
315+
COND_RC_MATCH = 1, /* the cond matched */
316+
COND_RC_STATUS_SET = 3 /* The condition eval set a final r->status */
317+
} cond_return_type;
318+
313319
typedef struct {
314320
char *input; /* Input string of RewriteCond */
315321
char *pattern; /* the RegExp pattern string */
@@ -4111,62 +4117,87 @@ static APR_INLINE int compare_lexicography(char *a, char *b)
41114117
/*
41124118
* Apply a single rewriteCond
41134119
*/
4114-
static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx, apr_pool_t *pool)
4120+
static cond_return_type apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx, apr_pool_t *pool)
41154121
{
41164122
char *input = NULL;
41174123
apr_finfo_t sb;
41184124
request_rec *rsub, *r = ctx->r;
41194125
ap_regmatch_t regmatch[AP_MAX_REG_MATCH];
4120-
int rc = 0;
4126+
int rc = COND_RC_NOMATCH;
41214127
int basis;
41224128

41234129
if (p->ptype != CONDPAT_AP_EXPR)
41244130
input = do_expand(p->input, ctx, NULL, NULL, pool);
41254131

41264132
switch (p->ptype) {
41274133
case CONDPAT_FILE_EXISTS:
4134+
if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
4135+
r->status = HTTP_FORBIDDEN;
4136+
rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
4137+
return COND_RC_STATUS_SET;
4138+
}
41284139
if ( apr_stat(&sb, input, APR_FINFO_MIN, r->pool) == APR_SUCCESS
41294140
&& sb.filetype == APR_REG) {
4130-
rc = 1;
4141+
rc = COND_RC_MATCH;
41314142
}
41324143
break;
41334144

41344145
case CONDPAT_FILE_SIZE:
4146+
if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
4147+
r->status = HTTP_FORBIDDEN;
4148+
rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
4149+
return COND_RC_STATUS_SET;
4150+
}
41354151
if ( apr_stat(&sb, input, APR_FINFO_MIN, r->pool) == APR_SUCCESS
41364152
&& sb.filetype == APR_REG && sb.size > 0) {
4137-
rc = 1;
4153+
rc = COND_RC_MATCH;
41384154
}
41394155
break;
41404156

41414157
case CONDPAT_FILE_LINK:
4158+
if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
4159+
r->status = HTTP_FORBIDDEN;
4160+
rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
4161+
return COND_RC_STATUS_SET;
4162+
}
41424163
#if !defined(OS2)
41434164
if ( apr_stat(&sb, input, APR_FINFO_MIN | APR_FINFO_LINK,
41444165
r->pool) == APR_SUCCESS
41454166
&& sb.filetype == APR_LNK) {
4146-
rc = 1;
4167+
rc = COND_RC_MATCH;
41474168
}
41484169
#endif
41494170
break;
41504171

41514172
case CONDPAT_FILE_DIR:
4173+
if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
4174+
r->status = HTTP_FORBIDDEN;
4175+
rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
4176+
return COND_RC_STATUS_SET;
4177+
}
41524178
if ( apr_stat(&sb, input, APR_FINFO_MIN, r->pool) == APR_SUCCESS
41534179
&& sb.filetype == APR_DIR) {
4154-
rc = 1;
4180+
rc = COND_RC_MATCH;
41554181
}
41564182
break;
41574183

41584184
case CONDPAT_FILE_XBIT:
4185+
if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
4186+
r->status = HTTP_FORBIDDEN;
4187+
rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
4188+
return COND_RC_STATUS_SET;
4189+
}
41594190
if ( apr_stat(&sb, input, APR_FINFO_PROT, r->pool) == APR_SUCCESS
41604191
&& (sb.protection & (APR_UEXECUTE | APR_GEXECUTE | APR_WEXECUTE))) {
4161-
rc = 1;
4192+
rc = COND_RC_MATCH;
41624193
}
41634194
break;
41644195

41654196
case CONDPAT_LU_URL:
41664197
if (*input && subreq_ok(r)) {
41674198
rsub = ap_sub_req_lookup_uri(input, r, NULL);
41684199
if (rsub->status < 400) {
4169-
rc = 1;
4200+
rc = COND_RC_MATCH;
41704201
}
41714202
rewritelog(r, 5, NULL, "RewriteCond URI (-U check: "
41724203
"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
41764207

41774208
case CONDPAT_LU_FILE:
41784209
if (*input && subreq_ok(r)) {
4210+
if (APR_SUCCESS != ap_stat_check(input, r->pool)) {
4211+
r->status = HTTP_FORBIDDEN;
4212+
rewritelog(r, 4, ctx->perdir, "RewriteCond: refusing to stat input='%s'", input);
4213+
return COND_RC_STATUS_SET;
4214+
}
41794215
rsub = ap_sub_req_lookup_file(input, r, NULL);
41804216
if (rsub->status < 300 &&
41814217
/* double-check that file exists since default result is 200 */
41824218
apr_stat(&sb, rsub->filename, APR_FINFO_MIN,
41834219
r->pool) == APR_SUCCESS) {
4184-
rc = 1;
4220+
rc = COND_RC_MATCH;
41854221
}
41864222
rewritelog(r, 5, NULL, "RewriteCond file (-F check: path=%s "
41874223
"-> 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
41974233
basis = 1;
41984234
test_str_g:
41994235
if (p->flags & CONDFLAG_NOCASE) {
4200-
rc = (strcasecmp(input, p->pattern) >= basis) ? 1 : 0;
4236+
rc = (strcasecmp(input, p->pattern) >= basis) ? COND_RC_MATCH : COND_RC_NOMATCH;
42014237
}
42024238
else {
4203-
rc = (compare_lexicography(input, p->pattern) >= basis) ? 1 : 0;
4239+
rc = (compare_lexicography(input, p->pattern) >= basis) ? COND_RC_MATCH : COND_RC_NOMATCH;
42044240
}
42054241
break;
42064242

@@ -4211,10 +4247,10 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx, apr_pool_t
42114247
basis = -1;
42124248
test_str_l:
42134249
if (p->flags & CONDFLAG_NOCASE) {
4214-
rc = (strcasecmp(input, p->pattern) <= basis) ? 1 : 0;
4250+
rc = (strcasecmp(input, p->pattern) <= basis) ? COND_RC_MATCH : COND_RC_NOMATCH;
42154251
}
42164252
else {
4217-
rc = (compare_lexicography(input, p->pattern) <= basis) ? 1 : 0;
4253+
rc = (compare_lexicography(input, p->pattern) <= basis) ? COND_RC_MATCH : COND_RC_NOMATCH;
42184254
}
42194255
break;
42204256

@@ -4245,7 +4281,10 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx, apr_pool_t
42454281
rewritelog(r, 1, ctx->perdir,
42464282
"RewriteCond: expr='%s' evaluation failed: %s",
42474283
p->pattern - p->pskip, err);
4248-
rc = 0;
4284+
rc = COND_RC_NOMATCH;
4285+
}
4286+
else {
4287+
rc = COND_RC_MATCH;
42494288
}
42504289
/* update briRC backref info */
42514290
if (rc && !(p->flags & CONDFLAG_NOTMATCH)) {
@@ -4266,7 +4305,7 @@ static int apply_rewrite_cond(rewritecond_entry *p, rewrite_ctx *ctx, apr_pool_t
42664305
break;
42674306
}
42684307

4269-
if (p->flags & CONDFLAG_NOTMATCH) {
4308+
if (p->flags & CONDFLAG_NOTMATCH && rc <= COND_RC_MATCH) {
42704309
rc = !rc;
42714310
}
42724311

@@ -4399,6 +4438,12 @@ static rule_return_type apply_rewrite_rule(rewriterule_entry *p,
43994438
rewritecond_entry *c = &conds[i];
44004439

44014440
rc = apply_rewrite_cond(c, ctx, ctx->temp_pool ? ctx->temp_pool : r->pool);
4441+
4442+
/* Error while evaluating cond, r->status set */
4443+
if (COND_RC_STATUS_SET == rc) {
4444+
return RULE_RC_STATUS_SET;
4445+
}
4446+
44024447
/*
44034448
* Reset vary_this if the novary flag is set for this condition.
44044449
*/

‎server/core.c‎

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

58865886
static apr_status_t core_dirwalk_stat(apr_finfo_t *finfo, request_rec *r,
5887-
apr_int32_t wanted)
5887+
apr_int32_t wanted)
58885888
{
5889-
return apr_stat(finfo, r->filename, wanted, r->pool);
5889+
apr_status_t rv = ap_stat_check(r->filename, r->pool);
5890+
if (rv == APR_SUCCESS) {
5891+
rv = apr_stat(finfo, r->filename, wanted, r->pool);
5892+
}
5893+
return rv;
58905894
}
58915895

58925896
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)
60476051
*s++ = '/';
60486052

60496053
ap_log_error(APLOG_MARK, APLOG_TRACE4, 0, ap_server_conf,
6050-
"ap_filepath_merge: check converted path %s allowed %d",
6054+
"check_unc: check converted path %s allowed %d",
60516055
teststring,
60526056
sconf->unc_list ? sconf->unc_list->nelts : 0);
60536057

@@ -6059,19 +6063,19 @@ static apr_status_t check_unc(const char *path, apr_pool_t *p)
60596063
!ap_cstr_casecmp(uri.hostinfo, configured_unc))) {
60606064
rv = APR_SUCCESS;
60616065
ap_log_error(APLOG_MARK, APLOG_TRACE4, 0, ap_server_conf,
6062-
"ap_filepath_merge: match %s %s",
6066+
"check_unc: match %s %s",
60636067
uri.hostinfo, configured_unc);
60646068
break;
60656069
}
60666070
else {
60676071
ap_log_error(APLOG_MARK, APLOG_TRACE4, 0, ap_server_conf,
6068-
"ap_filepath_merge: no match %s %s", uri.hostinfo,
6072+
"check_unc: no match %s %s", uri.hostinfo,
60696073
configured_unc);
60706074
}
60716075
}
60726076
if (rv != APR_SUCCESS) {
60736077
ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf, APLOGNO(10504)
6074-
"ap_filepath_merge: UNC path %s not allowed by UNCList", teststring);
6078+
"check_unc: UNC path %s not allowed by UNCList", teststring);
60756079
}
60766080

60776081
return rv;
@@ -6101,6 +6105,17 @@ AP_DECLARE(apr_status_t) ap_filepath_merge(char **newpath,
61016105
#endif
61026106
}
61036107

6108+
#ifdef WIN32
6109+
AP_DECLARE(apr_status_t) ap_stat_check(const char *path, apr_pool_t *p)
6110+
{
6111+
return check_unc(path, p);
6112+
}
6113+
#else
6114+
AP_DECLARE(apr_status_t) ap_stat_check(const char *path, apr_pool_t *p)
6115+
{
6116+
return APR_SUCCESS;
6117+
}
6118+
#endif
61046119

61056120
static void register_hooks(apr_pool_t *p)
61066121
{

‎server/util_expr_eval.c‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1527,10 +1527,21 @@ static const char *file_func(ap_expr_eval_ctx_t *ctx, const void *data,
15271527
return buf;
15281528
}
15291529

1530+
static apr_status_t stat_check(ap_expr_eval_ctx_t *ctx, const void *data, const char *arg)
1531+
{
1532+
apr_status_t rv = APR_SUCCESS;
1533+
if (APR_SUCCESS != (rv = ap_stat_check(arg, ctx->p))) {
1534+
*ctx->err = apr_psprintf(ctx->p, "stat of %s not allowed", arg);
1535+
}
1536+
return rv;
1537+
}
15301538
static const char *filesize_func(ap_expr_eval_ctx_t *ctx, const void *data,
15311539
char *arg)
15321540
{
15331541
apr_finfo_t sb;
1542+
if (APR_SUCCESS != stat_check(ctx, data, arg)) {
1543+
return "";
1544+
}
15341545
if (apr_stat(&sb, arg, APR_FINFO_MIN, ctx->p) == APR_SUCCESS
15351546
&& sb.filetype == APR_REG && sb.size > 0)
15361547
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,
15421553
char *arg)
15431554
{
15441555
apr_finfo_t sb;
1556+
if (APR_SUCCESS != stat_check(ctx, data, arg)) {
1557+
return "";
1558+
}
15451559
if (apr_stat(&sb, arg, APR_FINFO_MIN, ctx->p) == APR_SUCCESS
15461560
&& sb.filetype == APR_REG && sb.mtime > 0)
15471561
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
15771591
{
15781592
apr_finfo_t sb;
15791593
const char *name = (const char *)data;
1594+
if (APR_SUCCESS != stat_check(ctx, data, arg)) {
1595+
return FALSE;
1596+
}
15801597
if (apr_stat(&sb, arg, APR_FINFO_MIN, ctx->p) != APR_SUCCESS)
15811598
return FALSE;
15821599
switch (name[0]) {
@@ -1598,6 +1615,9 @@ static int op_file_link(ap_expr_eval_ctx_t *ctx, const void *data, const char *a
15981615
{
15991616
#if !defined(OS2)
16001617
apr_finfo_t sb;
1618+
if (APR_SUCCESS != stat_check(ctx, data, arg)) {
1619+
return FALSE;
1620+
}
16011621
if (apr_stat(&sb, arg, APR_FINFO_MIN | APR_FINFO_LINK, ctx->p) == APR_SUCCESS
16021622
&& sb.filetype == APR_LNK) {
16031623
return TRUE;
@@ -1609,6 +1629,9 @@ static int op_file_link(ap_expr_eval_ctx_t *ctx, const void *data, const char *a
16091629
static int op_file_xbit(ap_expr_eval_ctx_t *ctx, const void *data, const char *arg)
16101630
{
16111631
apr_finfo_t sb;
1632+
if (APR_SUCCESS != stat_check(ctx, data, arg)) {
1633+
return FALSE;
1634+
}
16121635
if (apr_stat(&sb, arg, APR_FINFO_PROT| APR_FINFO_LINK, ctx->p) == APR_SUCCESS
16131636
&& (sb.protection & (APR_UEXECUTE | APR_GEXECUTE | APR_WEXECUTE))) {
16141637
return TRUE;
@@ -1645,6 +1668,9 @@ static int op_file_subr(ap_expr_eval_ctx_t *ctx, const void *data, const char *a
16451668
request_rec *rsub, *r = ctx->r;
16461669
if (!r)
16471670
return FALSE;
1671+
if (APR_SUCCESS != stat_check(ctx, data, arg)) {
1672+
return FALSE;
1673+
}
16481674
rsub = ap_sub_req_lookup_file(arg, r, NULL);
16491675
if (rsub->status < 300 &&
16501676
/* double-check that file exists since default result is 200 */

0 commit comments

Comments
 (0)