perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Damon Buckwalter <buck...@gmail.com>
Subject Re: ACL filesystem incompatibility and potential race condition
Date Thu, 03 Mar 2005 07:00:30 GMT
On Wed, 02 Mar 2005 21:06:18 -0500, Stas Bekman <stas@stason.org> wrote:
> Damon Buckwalter wrote:
> > On Wed, 02 Mar 2005 18:32:40 -0500, Stas Bekman <stas@stason.org> wrote:
> >
> > I managed to come up with something very similar to this, minus the
> > last bit.  I like it, but I did manage to (partially) implement some
> > code on the C/XS side to return different statuses based on whether
> > the file is unreadable or non-existent.  It is broken because I can't
> > figure out how to return the script from modperl_slurp_filehandle() to
> > $self->{CODE}.  I just started learning XS today ;^)...  I was able to
> > make missing or unreadable files return a proper status code at least.
> 
> You don't need to do any XS changes. Just the following will do:
> 
>      $self->{CODE} = eval { $self->{REQ}->slurp_filename(0) }; # untainted
>      if ($@) {
>          $self->log_error("$@");
>          $rc = $@ == APR::EACCES ? Apache::FORBIDDEN : Apache::NOT_FOUND;
>      }
> 
> $@ is a magic object under mod_perl 2. Take a look at:
> http://perl.apache.org/docs/2.0/api/APR/Error.html

This produces the following for scripts that cannot be read/do not exist:

Software error:

Argument "Error opening '/home/www/foo.cgi...." isn't numeric in
numeric eq (==) at /usr/lib/perl5/Apache2/ModPerl/RegistryCooker.pm
line 543.

Looking at the APR::Error docs, I changed this to something like:

$rc = ref $@ eq 'APR::Error' && $@ == APR::EACCES ? Apache::FORBIDDEN
: Apache::NOT_FOUND

But I only ever get NOT_FOUND, even for files that exist but do not
have read permisson.

> Your case is certainly faulty:
> 
> +    if ($rc == APR::EACCES) {
> +        return Apache::DECLINED;
> 
> why declined and not forbidden?

This was just a guess on my part to try and make a functional proof-of-concept.

> 
> +    } elsif ($rc == APR::ENOENT) {
> +        return Apache::NOT_FOUND;
> +    } else {
> +        return Apache::OK;
> 
> why default to Apache::OK, which may miss some other errors you didn't
> encounter for. We should either default to Apache::NOT_FOUND or 500, but
> see below.

Right, this is also broken.  It would be nice to differentiate between
non-existence of a script versus bad permissions if we have that
information, but not really necessary.

> 
> Further APR::EACCES will be given no matter whether the file exists or its
> perms are not right. So you will never end up with Apache::NOT_FOUND.
> 
> You can see that tests like t/404.t are failing with it, when using this
> version in addition to my last patch:
> 
> sub read_script {
>      my $self = shift;
>      my $rc = Apache::OK;
> 
>      $self->debug("reading $self->{FILENAME}") if DEBUG & D_NOISE;
>      $self->{CODE} = eval { $self->{REQ}->slurp_filename(0) }; # untainted
>      if ($@) {
>          $rc = $@ == APR::EACCES ? Apache::FORBIDDEN : Apache::NOT_FOUND;
>          $self->log_error("$@");
>      }
> 
>      return $rc;
> }
> 
> I'd rather always return Apache::NOT_FOUND, and log_error() logs the right
> error message. I don't think the error code makes any difference to the
> user, when they can't reach the page.

I think it is a bit confusing to return 404 if the script is there,
just not readable.

> regarding changing the slurp_filename() API: passing a buffer by reference
> is ugly and not perlish (though we do it sometimes where we have no
> choice). Here $@ gives us the exact error code and stringified error
> message if wanted. Ideally it should have been $!, but unfortunately it's
> impossible to extend perl-core functionality on this one. Hence the $@ object.

I was just looking for a way to propagate the return code from
apr_file_open out to modperl, so I could see why it failed.  If $@
achieves the same goal, it works for me.

Thanks for your work on this,
Damon

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Mime
View raw message