Mailing List Archive

Does this deal with .. in the path? I'm not saying it doesn't, just wondering.

>
> I've uploaded a patch for this:
>
> 31_include.0.8.15.patch
> -----------------------
>
> Subject: SSI #include file="/foo/bar" shouldn't work
> Affects: mod_include.c
> ChangeLog: Disallow includes of files not in the same directory as the
> .shtml file.
>
> *** mod_include.c.orig Tue Oct 10 23:00:31 1995
> --- mod_include.c Thu Oct 19 12:24:16 1995
> ***************
> *** 360,372 ****
> if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
> return 1;
> if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
> ! request_rec *rr =
> ! (tag[0] == 'f')?
> ! sub_req_lookup_file (tag_val, r) :
> ! sub_req_lookup_uri (tag_val, r);
> char *error_fmt = NULL;
>
> ! if (rr->status != 200)
> error_fmt = "unable to include %s in parsed file %s";
>
> if (!error_fmt && noexec && rr->content_type
> --- 360,378 ----
> if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
> return 1;
> if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
> ! request_rec *rr=NULL;
> char *error_fmt = NULL;
> +
> + if (tag[0] == 'f')
> + { /* be safe; only files in this directory allowed */
> + if (strchr(tag_val, '/') != NULL)
> + error_fmt = "unable to include file %s in parsed file %s";
> + else
> + rr = sub_req_lookup_file (tag_val, r);
> + } else
> + rr = sub_req_lookup_uri (tag_val, r);
>
> ! if (!error_fmt && rr->status != 200)
> error_fmt = "unable to include %s in parsed file %s";
>
> if (!error_fmt && noexec && rr->content_type
> ***************
> *** 383,389 ****
> rprintf(r,"%s",error);
> }
>
> ! destroy_sub_req (rr);
> }
> else if(!strcmp(tag,"done"))
> return 0;
> --- 389,395 ----
> rprintf(r,"%s",error);
> }
>
> ! if (rr != NULL) destroy_sub_req (rr);
> }
> else if(!strcmp(tag,"done"))
> return 0;

--
Ben Laurie Phone: +44 (181) 994 6435
Freelance Consultant Fax: +44 (181) 994 6472
and Technical Director Email: ben@algroup.co.uk
A.L. Digital Ltd,
London, England.

Does this deal with .. in the path? I'm not saying it doesn't, just wondering.

>
> I've uploaded a patch for this:
>
> 31_include.0.8.15.patch
> -----------------------
>
> Subject: SSI #include file="/foo/bar" shouldn't work
> Affects: mod_include.c
> ChangeLog: Disallow includes of files not in the same directory as the
> .shtml file.
>
> *** mod_include.c.orig Tue Oct 10 23:00:31 1995
> --- mod_include.c Thu Oct 19 12:24:16 1995
> ***************
> *** 360,372 ****
> if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
> return 1;
> if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
> ! request_rec *rr =
> ! (tag[0] == 'f')?
> ! sub_req_lookup_file (tag_val, r) :
> ! sub_req_lookup_uri (tag_val, r);
> char *error_fmt = NULL;
>
> ! if (rr->status != 200)
> error_fmt = "unable to include %s in parsed file %s";
>
> if (!error_fmt && noexec && rr->content_type
> --- 360,378 ----
> if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
> return 1;
> if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
> ! request_rec *rr=NULL;
> char *error_fmt = NULL;
> +
> + if (tag[0] == 'f')
> + { /* be safe; only files in this directory allowed */
> + if (strchr(tag_val, '/') != NULL)
> + error_fmt = "unable to include file %s in parsed file %s";
> + else
> + rr = sub_req_lookup_file (tag_val, r);
> + } else
> + rr = sub_req_lookup_uri (tag_val, r);
>
> ! if (!error_fmt && rr->status != 200)
> error_fmt = "unable to include %s in parsed file %s";
>
> if (!error_fmt && noexec && rr->content_type
> ***************
> *** 383,389 ****
> rprintf(r,"%s",error);
> }
>
> ! destroy_sub_req (rr);
> }
> else if(!strcmp(tag,"done"))
> return 0;
> --- 389,395 ----
> rprintf(r,"%s",error);
> }
>
> ! if (rr != NULL) destroy_sub_req (rr);
> }
> else if(!strcmp(tag,"done"))
> return 0;

--
Ben Laurie Phone: +44 (181) 994 6435
Freelance Consultant Fax: +44 (181) 994 6472
and Technical Director Email: ben@algroup.co.uk
A.L. Digital Ltd,
London, England.

Does this deal with .. in the path? I'm not saying it doesn't, just wondering.

>
> I've uploaded a patch for this:
>
> 31_include.0.8.15.patch
> -----------------------
>
> Subject: SSI #include file="/foo/bar" shouldn't work
> Affects: mod_include.c
> ChangeLog: Disallow includes of files not in the same directory as the
> .shtml file.
>
> *** mod_include.c.orig Tue Oct 10 23:00:31 1995
> --- mod_include.c Thu Oct 19 12:24:16 1995
> ***************
> *** 360,372 ****
> if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
> return 1;
> if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
> ! request_rec *rr =
> ! (tag[0] == 'f')?
> ! sub_req_lookup_file (tag_val, r) :
> ! sub_req_lookup_uri (tag_val, r);
> char *error_fmt = NULL;
>
> ! if (rr->status != 200)
> error_fmt = "unable to include %s in parsed file %s";
>
> if (!error_fmt && noexec && rr->content_type
> --- 360,378 ----
> if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
> return 1;
> if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
> ! request_rec *rr=NULL;
> char *error_fmt = NULL;
> +
> + if (tag[0] == 'f')
> + { /* be safe; only files in this directory allowed */
> + if (strchr(tag_val, '/') != NULL)
> + error_fmt = "unable to include file %s in parsed file %s";
> + else
> + rr = sub_req_lookup_file (tag_val, r);
> + } else
> + rr = sub_req_lookup_uri (tag_val, r);
>
> ! if (!error_fmt && rr->status != 200)
> error_fmt = "unable to include %s in parsed file %s";
>
> if (!error_fmt && noexec && rr->content_type
> ***************
> *** 383,389 ****
> rprintf(r,"%s",error);
> }
>
> ! destroy_sub_req (rr);
> }
> else if(!strcmp(tag,"done"))
> return 0;
> --- 389,395 ----
> rprintf(r,"%s",error);
> }
>
> ! if (rr != NULL) destroy_sub_req (rr);
> }
> else if(!strcmp(tag,"done"))
> return 0;

--
Ben Laurie Phone: +44 (181) 994 6435
Freelance Consultant Fax: +44 (181) 994 6472
and Technical Director Email: ben@algroup.co.uk
A.L. Digital Ltd,
London, England.

Does this deal with .. in the path? I'm not saying it doesn't, just wondering.

>
> I've uploaded a patch for this:
>
> 31_include.0.8.15.patch
> -----------------------
>
> Subject: SSI #include file="/foo/bar" shouldn't work
> Affects: mod_include.c
> ChangeLog: Disallow includes of files not in the same directory as the
> .shtml file.
>
> *** mod_include.c.orig Tue Oct 10 23:00:31 1995
> --- mod_include.c Thu Oct 19 12:24:16 1995
> ***************
> *** 360,372 ****
> if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
> return 1;
> if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
> ! request_rec *rr =
> ! (tag[0] == 'f')?
> ! sub_req_lookup_file (tag_val, r) :
> ! sub_req_lookup_uri (tag_val, r);
> char *error_fmt = NULL;
>
> ! if (rr->status != 200)
> error_fmt = "unable to include %s in parsed file %s";
>
> if (!error_fmt && noexec && rr->content_type
> --- 360,378 ----
> if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
> return 1;
> if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
> ! request_rec *rr=NULL;
> char *error_fmt = NULL;
> +
> + if (tag[0] == 'f')
> + { /* be safe; only files in this directory allowed */
> + if (strchr(tag_val, '/') != NULL)
> + error_fmt = "unable to include file %s in parsed file %s";
> + else
> + rr = sub_req_lookup_file (tag_val, r);
> + } else
> + rr = sub_req_lookup_uri (tag_val, r);
>
> ! if (!error_fmt && rr->status != 200)
> error_fmt = "unable to include %s in parsed file %s";
>
> if (!error_fmt && noexec && rr->content_type
> ***************
> *** 383,389 ****
> rprintf(r,"%s",error);
> }
>
> ! destroy_sub_req (rr);
> }
> else if(!strcmp(tag,"done"))
> return 0;
> --- 389,395 ----
> rprintf(r,"%s",error);
> }
>
> ! if (rr != NULL) destroy_sub_req (rr);
> }
> else if(!strcmp(tag,"done"))
> return 0;

--
Ben Laurie Phone: +44 (181) 994 6435
Freelance Consultant Fax: +44 (181) 994 6472
and Technical Director Email: ben@algroup.co.uk
A.L. Digital Ltd,
London, England.

As I have understood and used it:

file="..." is a path (absolute or relative) to an included document.
virtual="..." uses DOCUMENTROOT for the active server and *must* begin
from the DOCUMENTROOT.

From a security standpoint, I would prefer to preserve the functionality
above and perhaps restrict file="..." to be a file within the DOCUMENTROOT
filespace. Use of FollowSymlink etc. should be our controls of this
filespace.


> Just for the record, because I think it's not something that's terribly
> well documented in any doc I've ever seen, and I don't want to be further
> confused by other people's interpretations - What is the difference, as we
> understand it, between:
>
> #include file="....."
>
> and
> #include virtual="....."
>
> Suggestions:
>
> file "....." can be in SAME directory as including file
>
> file="local_header.html"
>
> "....." can be in subdirectories
>
> file="Way/Down/There/foo.html"
>
> "....." can NOT be anywhere else
>
> file="/This/Is/Just/Plain/r0ng.html"
> file="../../As/Is/thi5.html" <-- YOU NEED TO CHECK THIS TOO!
> IF WE'RE TO BE NCSA 1.3R
> COMPATIBLE
>
> virtual "....." can be anywhere in UNIX space eg:
>
> virtual="/etc/passwd"
>
> "....." can be anywhere in document space
>
> virtual="../../Admin/default_copyright.html"
>
>
> References:
>
> http://hoohoo.ncsa.uiuc.edu/docs/tutorials/includes.html
>
>
> SUMMARY
>
> It's still broken and this patch will hurt people ;)
>
> Cheers,
> Ay.

As I have understood and used it:

file="..." is a path (absolute or relative) to an included document.
virtual="..." uses DOCUMENTROOT for the active server and *must* begin
from the DOCUMENTROOT.

From a security standpoint, I would prefer to preserve the functionality
above and perhaps restrict file="..." to be a file within the DOCUMENTROOT
filespace. Use of FollowSymlink etc. should be our controls of this
filespace.


> Just for the record, because I think it's not something that's terribly
> well documented in any doc I've ever seen, and I don't want to be further
> confused by other people's interpretations - What is the difference, as we
> understand it, between:
>
> #include file="....."
>
> and
> #include virtual="....."
>
> Suggestions:
>
> file "....." can be in SAME directory as including file
>
> file="local_header.html"
>
> "....." can be in subdirectories
>
> file="Way/Down/There/foo.html"
>
> "....." can NOT be anywhere else
>
> file="/This/Is/Just/Plain/r0ng.html"
> file="../../As/Is/thi5.html" <-- YOU NEED TO CHECK THIS TOO!
> IF WE'RE TO BE NCSA 1.3R
> COMPATIBLE
>
> virtual "....." can be anywhere in UNIX space eg:
>
> virtual="/etc/passwd"
>
> "....." can be anywhere in document space
>
> virtual="../../Admin/default_copyright.html"
>
>
> References:
>
> http://hoohoo.ncsa.uiuc.edu/docs/tutorials/includes.html
>
>
> SUMMARY
>
> It's still broken and this patch will hurt people ;)
>
> Cheers,
> Ay.

I've uploaded a patch for this:

31_include.0.8.15.patch
-----------------------

Subject: SSI #include file="/foo/bar" shouldn't work
Affects: mod_include.c
ChangeLog: Disallow includes of files not in the same directory as the
.shtml file.

*** mod_include.c.orig Tue Oct 10 23:00:31 1995
--- mod_include.c Thu Oct 19 12:24:16 1995
***************
*** 360,372 ****
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr =
! (tag[0] == 'f')?
! sub_req_lookup_file (tag_val, r) :
! sub_req_lookup_uri (tag_val, r);
char *error_fmt = NULL;

! if (rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
--- 360,378 ----
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr=NULL;
char *error_fmt = NULL;
+
+ if (tag[0] == 'f')
+ { /* be safe; only files in this directory allowed */
+ if (strchr(tag_val, '/') != NULL)
+ error_fmt = "unable to include file %s in parsed file %s";
+ else
+ rr = sub_req_lookup_file (tag_val, r);
+ } else
+ rr = sub_req_lookup_uri (tag_val, r);

! if (!error_fmt && rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
***************
*** 383,389 ****
rprintf(r,"%s",error);
}

! destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;
--- 389,395 ----
rprintf(r,"%s",error);
}

! if (rr != NULL) destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;

I've uploaded a patch for this:

31_include.0.8.15.patch
-----------------------

Subject: SSI #include file="/foo/bar" shouldn't work
Affects: mod_include.c
ChangeLog: Disallow includes of files not in the same directory as the
.shtml file.

*** mod_include.c.orig Tue Oct 10 23:00:31 1995
--- mod_include.c Thu Oct 19 12:24:16 1995
***************
*** 360,372 ****
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr =
! (tag[0] == 'f')?
! sub_req_lookup_file (tag_val, r) :
! sub_req_lookup_uri (tag_val, r);
char *error_fmt = NULL;

! if (rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
--- 360,378 ----
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr=NULL;
char *error_fmt = NULL;
+
+ if (tag[0] == 'f')
+ { /* be safe; only files in this directory allowed */
+ if (strchr(tag_val, '/') != NULL)
+ error_fmt = "unable to include file %s in parsed file %s";
+ else
+ rr = sub_req_lookup_file (tag_val, r);
+ } else
+ rr = sub_req_lookup_uri (tag_val, r);

! if (!error_fmt && rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
***************
*** 383,389 ****
rprintf(r,"%s",error);
}

! destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;
--- 389,395 ----
rprintf(r,"%s",error);
}

! if (rr != NULL) destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;

I've uploaded a patch for this:

31_include.0.8.15.patch
-----------------------

Subject: SSI #include file="/foo/bar" shouldn't work
Affects: mod_include.c
ChangeLog: Disallow includes of files not in the same directory as the
.shtml file.

*** mod_include.c.orig Tue Oct 10 23:00:31 1995
--- mod_include.c Thu Oct 19 12:24:16 1995
***************
*** 360,372 ****
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr =
! (tag[0] == 'f')?
! sub_req_lookup_file (tag_val, r) :
! sub_req_lookup_uri (tag_val, r);
char *error_fmt = NULL;

! if (rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
--- 360,378 ----
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr=NULL;
char *error_fmt = NULL;
+
+ if (tag[0] == 'f')
+ { /* be safe; only files in this directory allowed */
+ if (strchr(tag_val, '/') != NULL)
+ error_fmt = "unable to include file %s in parsed file %s";
+ else
+ rr = sub_req_lookup_file (tag_val, r);
+ } else
+ rr = sub_req_lookup_uri (tag_val, r);

! if (!error_fmt && rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
***************
*** 383,389 ****
rprintf(r,"%s",error);
}

! destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;
--- 389,395 ----
rprintf(r,"%s",error);
}

! if (rr != NULL) destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;

I've uploaded a patch for this:

31_include.0.8.15.patch
-----------------------

Subject: SSI #include file="/foo/bar" shouldn't work
Affects: mod_include.c
ChangeLog: Disallow includes of files not in the same directory as the
.shtml file.

*** mod_include.c.orig Tue Oct 10 23:00:31 1995
--- mod_include.c Thu Oct 19 12:24:16 1995
***************
*** 360,372 ****
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr =
! (tag[0] == 'f')?
! sub_req_lookup_file (tag_val, r) :
! sub_req_lookup_uri (tag_val, r);
char *error_fmt = NULL;

! if (rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
--- 360,378 ----
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr=NULL;
char *error_fmt = NULL;
+
+ if (tag[0] == 'f')
+ { /* be safe; only files in this directory allowed */
+ if (strchr(tag_val, '/') != NULL)
+ error_fmt = "unable to include file %s in parsed file %s";
+ else
+ rr = sub_req_lookup_file (tag_val, r);
+ } else
+ rr = sub_req_lookup_uri (tag_val, r);

! if (!error_fmt && rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
***************
*** 383,389 ****
rprintf(r,"%s",error);
}

! destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;
--- 389,395 ----
rprintf(r,"%s",error);
}

! if (rr != NULL) destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;

I've uploaded a patch for this:

31_include.0.8.15.patch
-----------------------

Subject: SSI #include file="/foo/bar" shouldn't work
Affects: mod_include.c
ChangeLog: Disallow includes of files not in the same directory as the
.shtml file.

*** mod_include.c.orig Tue Oct 10 23:00:31 1995
--- mod_include.c Thu Oct 19 12:24:16 1995
***************
*** 360,372 ****
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr =
! (tag[0] == 'f')?
! sub_req_lookup_file (tag_val, r) :
! sub_req_lookup_uri (tag_val, r);
char *error_fmt = NULL;

! if (rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
--- 360,378 ----
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr=NULL;
char *error_fmt = NULL;
+
+ if (tag[0] == 'f')
+ { /* be safe; only files in this directory allowed */
+ if (strchr(tag_val, '/') != NULL)
+ error_fmt = "unable to include file %s in parsed file %s";
+ else
+ rr = sub_req_lookup_file (tag_val, r);
+ } else
+ rr = sub_req_lookup_uri (tag_val, r);

! if (!error_fmt && rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
***************
*** 383,389 ****
rprintf(r,"%s",error);
}

! destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;
--- 389,395 ----
rprintf(r,"%s",error);
}

! if (rr != NULL) destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;

> I've uploaded a patch for this:
>
> 31_include.0.8.15.patch
> -----------------------
>
> Subject: SSI #include file="/foo/bar" shouldn't work
> Affects: mod_include.c
> ChangeLog: Disallow includes of files not in the same directory as the
> .shtml file.

Wups. But I'm confused by the wording in the change log. It seems to imply
that you don't want to allow 'file' to operate on subdirectories...

Just for the record, because I think it's not something that's terribly
well documented in any doc I've ever seen, and I don't want to be further
confused by other people's interpretations - What is the difference, as we
understand it, between:

#include file="....."

and
#include virtual="....."

Suggestions:

file "....." can be in SAME directory as including file

file="local_header.html"

"....." can be in subdirectories

file="Way/Down/There/foo.html"

"....." can NOT be anywhere else

file="/This/Is/Just/Plain/r0ng.html"
file="../../As/Is/thi5.html" <-- YOU NEED TO CHECK THIS TOO!
IF WE'RE TO BE NCSA 1.3R
COMPATIBLE

virtual "....." can be anywhere in UNIX space eg:

virtual="/etc/passwd"

"....." can be anywhere in document space

virtual="../../Admin/default_copyright.html"


References:

http://hoohoo.ncsa.uiuc.edu/docs/tutorials/includes.html


SUMMARY

It's still broken and this patch will hurt people ;)

Cheers,
Ay.

New patch uploaded, which should conform to Andrew's description.
I misread the spec; the crappy netscape font made '../ cannot be used in this
pathname' look like '/ cannot be used in this pathname'
[Memo; make sure any dots are in <code>]

31a_include.0.8.15.patch
------------------------

Subject: SSI #include file="/foo/bar" shouldn't work
Affects: mod_include.c
ChangeLog: Disallow includes of files not in the same directory as, or
sub-directory of, the .shtml file.

*** mod_include.c.orig Tue Oct 10 23:00:31 1995
--- mod_include.c Thu Oct 19 14:39:35 1995
***************
*** 360,372 ****
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr =
! (tag[0] == 'f')?
! sub_req_lookup_file (tag_val, r) :
! sub_req_lookup_uri (tag_val, r);
char *error_fmt = NULL;

! if (rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
--- 360,380 ----
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr=NULL;
char *error_fmt = NULL;
+
+ if (tag[0] == 'f')
+ { /* be safe; only files in this directory or below allowed */
+ char tmp[MAX_STRING_LEN+2];
+ sprintf(tmp, "/%s/", tag_val);
+ if (tag_val[0] == '/' || strstr(tmp, "/../") != NULL)
+ error_fmt = "unable to include file %s in parsed file %s";
+ else
+ rr = sub_req_lookup_file (tag_val, r);
+ } else
+ rr = sub_req_lookup_uri (tag_val, r);

! if (!error_fmt && rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
***************
*** 383,389 ****
rprintf(r,"%s",error);
}

! destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;
--- 391,397 ----
rprintf(r,"%s",error);
}

! if (rr != NULL) destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;

New patch uploaded, which should conform to Andrew's description.
I misread the spec; the crappy netscape font made '../ cannot be used in this
pathname' look like '/ cannot be used in this pathname'
[Memo; make sure any dots are in <code>]

31a_include.0.8.15.patch
------------------------

Subject: SSI #include file="/foo/bar" shouldn't work
Affects: mod_include.c
ChangeLog: Disallow includes of files not in the same directory as, or
sub-directory of, the .shtml file.

*** mod_include.c.orig Tue Oct 10 23:00:31 1995
--- mod_include.c Thu Oct 19 14:39:35 1995
***************
*** 360,372 ****
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr =
! (tag[0] == 'f')?
! sub_req_lookup_file (tag_val, r) :
! sub_req_lookup_uri (tag_val, r);
char *error_fmt = NULL;

! if (rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
--- 360,380 ----
if(!(tag_val = get_tag(r->pool, in, tag, MAX_STRING_LEN, 1)))
return 1;
if(!strcmp(tag,"file") || !strcmp (tag, "virtual")) {
! request_rec *rr=NULL;
char *error_fmt = NULL;
+
+ if (tag[0] == 'f')
+ { /* be safe; only files in this directory or below allowed */
+ char tmp[MAX_STRING_LEN+2];
+ sprintf(tmp, "/%s/", tag_val);
+ if (tag_val[0] == '/' || strstr(tmp, "/../") != NULL)
+ error_fmt = "unable to include file %s in parsed file %s";
+ else
+ rr = sub_req_lookup_file (tag_val, r);
+ } else
+ rr = sub_req_lookup_uri (tag_val, r);

! if (!error_fmt && rr->status != 200)
error_fmt = "unable to include %s in parsed file %s";

if (!error_fmt && noexec && rr->content_type
***************
*** 383,389 ****
rprintf(r,"%s",error);
}

! destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;
--- 391,397 ----
rprintf(r,"%s",error);
}

! if (rr != NULL) destroy_sub_req (rr);
}
else if(!strcmp(tag,"done"))
return 0;

> Apache incompatibility with NCSA httpd, and possible security hole:
>
> Apache server-side includes allow #include file=arbitrary-path
> whereas NCSA only allows #include file=local-file
>
> 'arbitrary-path' is the name of any file on the system, whereas 'local-file'
> can only be the name of a file in the same directory as the included file.
> i.e. NCSA httpd does not allow '/' in 'local-file'.

Mmm, file="Sub/Directory/thing.html" good
file="/etc/passwd" bad

So '/' is allowed, but not as the first character. Right? also '..' is
not allowed anywhere in file.

> As web-admins are expecting the NCSA behaviour, we should either document
> this visibly, or fix it.

We should fix it, in preference to just documenting it and giving NCSA-happy
people another reason to not use Apache.

> David.

Ay.

/*
* "Re: Apache incompatibility" by Andrew Wilson <andrew@tees.elsevier.co.uk>
* written Thu, 19 Oct 95 21:37:10 BST
*
* Randy:
** As I have understood and used it:
**
** file="..." is a path (absolute or relative) to an included
** document. virtual="..." uses DOCUMENTROOT for the active server
** and *must* begin from the DOCUMENTROOT.
*
* Aaah, virtual is beginning to make a little more sense. You
* understand how confusing this is for people. They read what sparse
* documentation there is available on the web regarding this feature,
* kick off Apache to play with the include syntax and *WOW* Apache
* doesn't do what it sez it should.
*/

The documentation at
http://hoohoo.ncsa.uiuc.edu/docs/tutorials/includes.html (which is
very sparse and very vague) says that file="" is not an absolute path,
it must be relative to the directory containing the document and
cannot contain ../. That means that if /docroot/foo/bar/baz.html
includes file=foo/bar then the server should include
/docroot/foo/bar/foo/bar

virtual= is a virtual path and should have the server's name
translation applied to it first. Yes, most of the time it will come
from the document root, but there might also be an Alias that affects
the translation. So /foo/bar gets translated to /docroot/foo/bar, or
it gets translated to /aliased/bar if someone has aliased /foo to /aliased

--Rob

Randy:
> As I have understood and used it:
>
> file="..." is a path (absolute or relative) to an included document.
> virtual="..." uses DOCUMENTROOT for the active server and *must* begin
> from the DOCUMENTROOT.

Aaah, virtual is beginning to make a little more sense. You understand
how confusing this is for people. They read what sparse documentation
there is available on the web regarding this feature, kick off Apache
to play with the include syntax and *WOW* Apache doesn't do what it sez it
should.

> >From a security standpoint, I would prefer to preserve the functionality
> above and perhaps restrict file="..." to be a file within the DOCUMENTROOT
> filespace. Use of FollowSymlink etc. should be our controls of this
> filespace.

I do not think that we should change the default in ANY WAY from the way
NCSA 1.3R handles this. However an augmented meaning could be applied by
directives in the *.conf files. But... Apache should not do anything *new*
by default.

> Randy:
> > As I have understood and used it:
> >
> > file="..." is a path (absolute or relative) to an included document.
> > virtual="..." uses DOCUMENTROOT for the active server and *must* begin
> > from the DOCUMENTROOT.
>
> > >From a security standpoint, I would prefer to preserve the functionality
> > above and perhaps restrict file="..." to be a file within the DOCUMENTROOT
> > filespace. Use of FollowSymlink etc. should be our controls of this
> > filespace.
>
> I do not think that we should change the default in ANY WAY from the way
> NCSA 1.3R handles this. However an augmented meaning could be applied by
> directives in the *.conf files. But... Apache should not do anything *new*
> by default.

From Rob M's description of the original NCSA behavior, I see no problem
with the way things work at present. file="..." still works as it does
in NCSA. I like the Apache feature of being able to include from other
places in the document tree. To make it more secure, it would probably
be good to restrict file to stay within DOCUMENTROOT.

I see no reason to remove Apache functionality. Some people may even
be using Apache for this feature...

>As I have understood and used it:
>
>file="..." is a path (absolute or relative) to an included document.
>virtual="..." uses DOCUMENTROOT for the active server and *must* begin
>from the DOCUMENTROOT.

NCSA:
file= a relative path to a file in the same directory or a sub-dir ONLY
virtual= an absolute URL (i.e. starts from DocumentRoot)

Apache
file= a relative or absolute file to anywhere
virtual a _relative_ or absolute URL (so is always in the doucment tree


From a security standpoint, I would prefer to preserve the functionality
>above and perhaps restrict file="..." to be a file within the DOCUMENTROOT
>filespace. Use of FollowSymlink etc. should be our controls of this
>filespace.

You have that in Apache with the extra functionality of virtual=...
This extra functionality hurts nobody.

However, you also have the extra functionality of file=xxxx to point to
any file on the system. The suggestion is that sysadmins wouldn't expect
this, and might reasonably complain that they didn't know they were
allowing their users access to this feature when they switched to Apache.

David.

Randy wrote:
>> I do not think that we should change the default in ANY WAY from the way
>> NCSA 1.3R handles this. However an augmented meaning could be applied by
>> directives in the *.conf files. But... Apache should not do anything *new*
>> by default.
>
>>From Rob M's description of the original NCSA behavior, I see no problem
>with the way things work at present. file="..." still works as it does
>in NCSA. I like the Apache feature of being able to include from other
>places in the document tree. To make it more secure, it would probably
>be good to restrict file to stay within DOCUMENTROOT.
>
>I see no reason to remove Apache functionality. Some people may even
>be using Apache for this feature...

The feature you want, _with_ the security check you suggest, is already
available by virtual="../foobar/..". The question is, should it be available
via file="..." _without_ the security check?

David.