You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Marc Slemko <ma...@znep.com> on 1998/10/31 20:51:52 UTC
[PATCH] PR#3323: recursive includes
There are a few recursive includes still possible, including the trivial
one of including "/" in "/" (or any directory that is handled by mod_dir).
The below patch attempts to fix it, and works for all the cases I can
think of. Comments are appreciated; I'm not overly familiar with this
part (ie. all the things that impact r->filename and uri and how
they can interact).
If any module plays with r->filename outside of the filename translation
phase, this may not necessarily be enough to fix the problem for that
module. What mod_dir does is:
/* KLUDGE --- make the sub_req lookups happen in the right directory.
* Fixing this in the sub_req_lookup functions themselves is difficult,
* and would probably break virtual includes...
*/
if (r->filename[strlen(r->filename) - 1] != '/') {
r->filename = ap_pstrcat(r->pool, r->filename, "/", NULL);
}
With mod_dir, you can't (well, I can't see a way) to make a request
for a ever-changing URL that still is handled as the same filename
by mod_dir.
If you can do an ever changing URL plus a r->filename munged outside
of the filename translation at the same time, you can bypass
the below checks.
The other thing that may be wise, in general for all modules, is the
ability to put a configurable limit the depth of subrequests and/or
internal redirects.
Also note that it is possible that there are times when you _want_
recursive includes of the same file and your recursive includes know
when they get to a certain depth and stop properly. That, however,
goes a bit beyond so I'm not sure if it is worth worrying about.
Index: modules/standard/mod_include.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/modules/standard/mod_include.c,v
retrieving revision 1.105
diff -u -r1.105 mod_include.c
--- mod_include.c 1998/09/24 14:06:42 1.105
+++ mod_include.c 1998/10/31 19:47:05
@@ -688,13 +688,41 @@
"in parsed file %s";
}
if (error_fmt == NULL) {
+ /* try to avoid recursive includes. We do this by walking
+ * up the r->main list of subrequests, and at each level
+ * walking back through any internal redirects. At each
+ * step, we compare the filenames and the URIs.
+ *
+ * The filename comparison catches a recursive include
+ * with an ever-changing URL, eg.
+ * <!--#include virtual=
+ * "$REQUEST_URI/$QUERY_STRING?$QUERY_STRING/x"-->
+ * which, although they would eventually be caught because
+ * we have a limit on the length of files, etc., can
+ * recurse for a while.
+ *
+ * The URI comparison catches the case where the filename
+ * is changed while processing the request, so the
+ * current name is never the same as any previous one.
+ * This can happen with "DocumentRoot /foo" when you
+ * request "/" on the server and it includes "/".
+ * This only applies to modules such as mod_dir that
+ * (somewhat improperly) mess with r->filename outside
+ * of a filename translation phase.
+ */
+ int founddupe = 0;
request_rec *p;
+ for (p = r; p != NULL && !founddupe; p = p->main) {
+ request_rec *q;
+ for (q = p; q != NULL; q = q->prev) {
+ if ( (strcmp(q->filename, rr->filename) == 0) ||
+ (strcmp(q->uri, rr->uri) == 0) ){
+ founddupe = 1;
+ break;
+ }
+ }
+ }
- for (p = r; p != NULL; p = p->main) {
- if (strcmp(p->filename, rr->filename) == 0) {
- break;
- }
- }
if (p != NULL) {
error_fmt = "Recursive include of \"%s\" "
"in parsed file %s";
Re: [PATCH] PR#3323: recursive includes
Posted by Ben Laurie <be...@algroup.co.uk>.
+1 (untested).
Marc Slemko wrote:
>
> There are a few recursive includes still possible, including the trivial
> one of including "/" in "/" (or any directory that is handled by mod_dir).
>
> The below patch attempts to fix it, and works for all the cases I can
> think of. Comments are appreciated; I'm not overly familiar with this
> part (ie. all the things that impact r->filename and uri and how
> they can interact).
>
> If any module plays with r->filename outside of the filename translation
> phase, this may not necessarily be enough to fix the problem for that
> module. What mod_dir does is:
>
> /* KLUDGE --- make the sub_req lookups happen in the right directory.
> * Fixing this in the sub_req_lookup functions themselves is difficult,
> * and would probably break virtual includes...
> */
>
> if (r->filename[strlen(r->filename) - 1] != '/') {
> r->filename = ap_pstrcat(r->pool, r->filename, "/", NULL);
> }
>
> With mod_dir, you can't (well, I can't see a way) to make a request
> for a ever-changing URL that still is handled as the same filename
> by mod_dir.
>
> If you can do an ever changing URL plus a r->filename munged outside
> of the filename translation at the same time, you can bypass
> the below checks.
>
> The other thing that may be wise, in general for all modules, is the
> ability to put a configurable limit the depth of subrequests and/or
> internal redirects.
>
> Also note that it is possible that there are times when you _want_
> recursive includes of the same file and your recursive includes know
> when they get to a certain depth and stop properly. That, however,
> goes a bit beyond so I'm not sure if it is worth worrying about.
>
> Index: modules/standard/mod_include.c
> ===================================================================
> RCS file: /export/home/cvs/apache-1.3/src/modules/standard/mod_include.c,v
> retrieving revision 1.105
> diff -u -r1.105 mod_include.c
> --- mod_include.c 1998/09/24 14:06:42 1.105
> +++ mod_include.c 1998/10/31 19:47:05
> @@ -688,13 +688,41 @@
> "in parsed file %s";
> }
> if (error_fmt == NULL) {
> + /* try to avoid recursive includes. We do this by walking
> + * up the r->main list of subrequests, and at each level
> + * walking back through any internal redirects. At each
> + * step, we compare the filenames and the URIs.
> + *
> + * The filename comparison catches a recursive include
> + * with an ever-changing URL, eg.
> + * <!--#include virtual=
> + * "$REQUEST_URI/$QUERY_STRING?$QUERY_STRING/x"-->
> + * which, although they would eventually be caught because
> + * we have a limit on the length of files, etc., can
> + * recurse for a while.
> + *
> + * The URI comparison catches the case where the filename
> + * is changed while processing the request, so the
> + * current name is never the same as any previous one.
> + * This can happen with "DocumentRoot /foo" when you
> + * request "/" on the server and it includes "/".
> + * This only applies to modules such as mod_dir that
> + * (somewhat improperly) mess with r->filename outside
> + * of a filename translation phase.
> + */
> + int founddupe = 0;
> request_rec *p;
> + for (p = r; p != NULL && !founddupe; p = p->main) {
> + request_rec *q;
> + for (q = p; q != NULL; q = q->prev) {
> + if ( (strcmp(q->filename, rr->filename) == 0) ||
> + (strcmp(q->uri, rr->uri) == 0) ){
> + founddupe = 1;
> + break;
> + }
> + }
> + }
>
> - for (p = r; p != NULL; p = p->main) {
> - if (strcmp(p->filename, rr->filename) == 0) {
> - break;
> - }
> - }
> if (p != NULL) {
> error_fmt = "Recursive include of \"%s\" "
> "in parsed file %s";
--
Ben Laurie |Phone: +44 (181) 735 0686| Apache Group member
Freelance Consultant |Fax: +44 (181) 735 0689|http://www.apache.org/
and Technical Director|Email: ben@algroup.co.uk |
A.L. Digital Ltd, |Apache-SSL author http://www.apache-ssl.org/
London, England. |"Apache: TDG" http://www.ora.com/catalog/apache/