--- erotomek <[EMAIL PROTECTED]> wrote:
> --- Curtis Poe <[EMAIL PROTECTED]> wrote:
> >
> > You supplied some great information, however, your
> > example of plugging the security hole has a
> > security hole itself.  From the command line on any
> > *nix system, enter the following (assuming you
> > are not in root):
> > 
> >     cd \.\.
> > 
> > You quickly discover that the shell allows those
> > dots to be escaped.  In a url, that translates to
> > something like:
> 
> Yes, the shell, but the program I was commenting
> didn't use the shell. And even if it used, it would
> die if there was any '\' in the input anyway. Maybe I
> don't get your point, but my code:
> 
> > > die 'INSECURE $file !!!' if $file =~ /\.\./;
> > > if ($file =~ /^([-\w./]+)$/) { $file = $1 }
> > > else { die 'INSECURE $file !!!' }
> 
> won't pass any '\' or '%' from your examples. If
> anything, it's too paranoidal, it won't work with
> files like "some..file..name", and maybe:

D'oh!!  You're right.  I saw the first line of code and reacted on that.  I see that 
bad attempt
to escape double dots so often that I had a knee-jerk reaction :)  As you pointed out, 
you weren't
using the shell and the escaped double dots problem generally on system and backtick 
calls
(http://www.insecure.org/news/P55-07.txt).  Whoops!

That being said, I still have some concerns about your code.  From a maintenance 
standpoint, you
have a possibility of introduction of errors due to code that needs to be refactored. 
Specifically, you have the same die statement in two places.  This means a maintenance 
programmer
who comes along later and doesn't quite "get it" could more easily introduce bugs by 
fiddling with
code to make it work.

There are several ways to fix this.   Since you don't want the double dots, I assume 
that you
don't want reverse directory traversal.  If *all* you are looking for is a valid 
filename, the
following is relatively safe, though somewhat restrictive:

    my ( $good_file ) = $file =~ !/(\w+\.\w+)$!;
    if ( ! defined $good_file )
    {
        die "Insecure file: $file";
    }

If you want to allow the user to specify path info, that becomes more problematic.  
Since I never
allow users to do that, I can't say that I'm too familiar with it.  My first thought 
would be to
use Cwd::abs_path() to eliminate the double dots.  Then, if that works (you'd want to 
eval it),
you can check to see if it's an allowed path.  Not sure how comfortable I feel about 
that, though.

Cheers,
Curtis "Ovid" Poe

=====
"Ovid" on http://www.perlmonks.org/
Someone asked me how to count to 10 in Perl:
push@A,$_ for reverse q.e...q.n.;for(@A){$_=unpack(q|c|,$_);@a=split//;
shift@a;shift@a if $a[$[]eq$[;$_=join q||,@a};print $_,$/for reverse @A

__________________________________________________
Do You Yahoo!?
Yahoo! Greetings - Send FREE e-cards for every occasion!
http://greetings.yahoo.com

-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to