You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2002/12/12 08:05:54 UTC
cvs commit: httpd-2.0/server request.c
wrowe 2002/12/11 23:05:54
Modified: server request.c
Log:
Make the code simpler to follow, and perhaps clear up the follow-symlink
bug reports we have seen on bugzilla. e.g. 14206 etc.
Revision Changes Path
1.122 +23 -43 httpd-2.0/server/request.c
Index: request.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/request.c,v
retrieving revision 1.121
retrieving revision 1.122
diff -u -r1.121 -r1.122
--- request.c 3 Nov 2002 22:17:32 -0000 1.121
+++ request.c 12 Dec 2002 07:05:54 -0000 1.122
@@ -363,25 +363,6 @@
* they change, all the way down.
*/
-/*
- * We don't want people able to serve up pipes, or unix sockets, or other
- * scary things. Note that symlink tests are performed later.
- */
-static int check_safe_file(request_rec *r)
-{
-
- if (r->finfo.filetype == 0 /* doesn't exist */
- || r->finfo.filetype == APR_DIR
- || r->finfo.filetype == APR_REG
- || r->finfo.filetype == APR_LNK) {
- return OK;
- }
-
- ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
- "object is not a file, directory or symlink: %s",
- r->filename);
- return HTTP_FORBIDDEN;
-}
/*
* resolve_symlink must _always_ be called on an APR_LNK file type!
@@ -577,7 +558,7 @@
* with APR_ENOENT, knowing that the path is good.
*/
if (!r->finfo.filetype || r->finfo.filetype == APR_LNK) {
- apr_stat(&r->finfo, r->filename, APR_FINFO_MIN, r->pool);
+ rv = apr_stat(&r->finfo, r->filename, APR_FINFO_MIN, r->pool);
/* some OSs will return APR_SUCCESS/APR_REG if we stat
* a regular file but we have '/' at the end of the name;
@@ -586,11 +567,14 @@
*
* handle it the same everywhere by simulating a failure
* if it looks like a directory but really isn't
+ *
+ * Also reset if the stat failed, just for safety.
*/
- if (r->finfo.filetype &&
- r->finfo.filetype != APR_DIR &&
- r->filename[strlen(r->filename) - 1] == '/') {
- r->finfo.filetype = 0; /* forget what we learned */
+ if ((rv != APR_SUCCESS) ||
+ (r->finfo.filetype &&
+ (r->finfo.filetype != APR_DIR) &&
+ (r->filename[strlen(r->filename) - 1] == '/'))) {
+ r->finfo.filetype = 0; /* forget what we learned */
}
}
@@ -963,7 +947,7 @@
++seg_name;
/* If nothing remained but a '/' string, we are finished
- */
+ * XXX: NO WE ARE NOT!!! Now process this puppy!!! */
if (!*seg_name) {
break;
}
@@ -1018,11 +1002,6 @@
return r->status = HTTP_FORBIDDEN;
}
- if ((res = check_safe_file(r))) {
- r->status = res;
- return res;
- }
-
/* Fix up the path now if we have a name, and they don't agree
*/
if ((thisinfo.valid & APR_FINFO_NAME)
@@ -1045,21 +1024,22 @@
r->filename);
return r->status = res;
}
+ }
- /* Ok, we are done with the link's info, test the real target
+ /* Ok, we are done with the link's info, test the real target
+ */
+ if (thisinfo.filetype == APR_REG ||
+ thisinfo.filetype == APR_NOFILE) {
+ /* That was fun, nothing left for us here
*/
- if (thisinfo.filetype == APR_REG) {
- /* That was fun, nothing left for us here
- */
- break;
- }
- else if (thisinfo.filetype != APR_DIR) {
- ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
- "symlink doesn't point to a file or "
- "directory: %s",
- r->filename);
- return r->status = HTTP_FORBIDDEN;
- }
+ break;
+ }
+ else if (thisinfo.filetype != APR_DIR) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ "Forbidden: %s doesn't point to "
+ "a file or directory",
+ r->filename);
+ return r->status = HTTP_FORBIDDEN;
}
++seg;
Re: cvs commit: httpd-2.0/server request.c
Posted by Francis Daly <de...@daoine.org>.
On Thu, Dec 12, 2002 at 11:31:44AM -0500, Paul J. Reder wrote:
> wrowe@apache.org wrote:
>
> >wrowe 2002/12/11 23:05:54
> >
> > Modified: server request.c
> > Log:
> > Make the code simpler to follow, and perhaps clear up the
> > follow-symlink
> > bug reports we have seen on bugzilla. e.g. 14206 etc.
>
> Sorry to be the bearer of bad news but the problem reported in 14206 still
> occurs with this new code.
[snip]
> Now bring up your browser and request:
>
> http://your.machine.name:port/index.html
>
> You'll get a 403:forbidden error.
>
> http://your.machine.name:port/
>
> You'll get the page foo.html.
>
> I can spend more time tracking this if you want, but it won't be
> till this afternoon.
Can I point out the mail archived at
http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=103938644204488&w=2
sent on December 8th, which has some discussion of this issue.
It might give some pointers as to where to start looking.
The patch there probably won't apply cleanly any more, of course.
All the best,
f
--
Francis Daly deva@daoine.org
Re: cvs commit: httpd-2.0/server request.c
Posted by "Paul J. Reder" <re...@remulak.net>.
wrowe@apache.org wrote:
> wrowe 2002/12/11 23:05:54
>
> Modified: server request.c
> Log:
> Make the code simpler to follow, and perhaps clear up the follow-symlink
> bug reports we have seen on bugzilla. e.g. 14206 etc.
>
> Revision Changes Path
> 1.122 +23 -43 httpd-2.0/server/request.c
Sorry to be the bearer of bad news but the problem reported in 14206 still
occurs with this new code. All you have to do is the following:
In your htdocs directory:
mv index.html foo.html
ln -s foo.html index.html
In your httpd.conf:
# Note: Options should not allow FollowSymLinks
<Directory />
Options None
AllowOverride None
</Directory>
<Directory /home/Apache/htdocs>
Options None
AllowOverride None
Order deny,allow
Allow from all
</Directory>
Now bring up your browser and request:
http://your.machine.name:port/index.html
You'll get a 403:forbidden error.
http://your.machine.name:port/
You'll get the page foo.html.
I can spend more time tracking this if you want, but it won't be
till this afternoon.
--
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it. Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein