[Full-disclosure] Re: Directory traversal in source.php not fixed.

• Previous message: Rob Holland: "[Full-disclosure] pam_ldap/nss_ldap password leak in a master+slave+start_tls LDAP setup"
• Next in thread: Kaf Oseo: "[Full-disclosure] Re: Directory traversal in source.php not fixed."
• Reply: Kaf Oseo: "[Full-disclosure] Re: Directory traversal in source.php not fixed."
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ] [ attachment ]

Date: Mon, 4 Jul 2005 11:56:08 -0700
To: Kaf Oseo <kaf@szub.net>

On Mon, Jul 04, 2005 at 01:17:50PM -0400, Kaf Oseo wrote:
> Thank you for contacting me.
>
> I've corrected, as well as further attempted to *harden*, my Quick &
> Dirty PHPSource Printer (PHP script).
>
> The line referred below to is now:
>
> $file = (strstr($file_get, '..') == true) ? NULL : $file_get;

Actually, if I'm not mistaken again ;), it would be faster this way
while still getting strpos speed advantages:

$file = (strpos($file_get, '..') === false) ? $file_get : NULL;

Note how we're testing for false and negating now -- I earlier made the
mistake that (!(a === false)) is the same thing as (a === true), but
they aren't because of the type munging going on (I'm really not used to
php's type munging semantics).

I suggested === in my first proposed fix because php.net says to use it
to test the return value, but they aren't explicit that this will only
work on false. strpos is also faster than strstr, according to
php.net's manual of strstr.

I'll note that perl's index function is actually sane in that it uses a
_different_ integer value for not found than one that overlaps with the
valid set of found index positions:

       index STR,SUBSTR,POSITION
       index STR,SUBSTR
             The index function searches for one string within another, but
             without the wildcard-like behavior of a full regular-expression
             pattern match. It returns the position of the first occurrence
             of SUBSTR in STR at or after POSITION. If POSITION is omitted,
             starts searching from the beginning of the string. The return
             value is based at 0 (or whatever you've set the $[ variable
             to--but don't do that). *If the substring is not found, returns*
             *one less than the base, ordinarily "-1".*
                                                                                                                                      
Just another reason why perl's so much easier to code secure software
in. I hate having to learn poorly-thought-through functions in php just
because monkeys are taught to use it instead of a real scripting
language. *sigh*

>
> Script available here:
> http://guff.szub.net/quick-and-dirty-phpsource-printer/
>
> Source can be viewed here:
> http://guff.szub.net/wp-content/sourceprt.php?file=source.php
>
> -Kaf Oseo
>
> Chew Keong Tan wrote:
> >Hi,
> >
> >I have taken a look at source.php and the vulnerability does not seem to
> >be fixed. This is due to an error in the strstr comparison in the
> >following line of code. Further, if your script is deployed in the
> >Windows platform, then "..\" sequences can also be used for directory
> >traversal.

Unfortunate if this is true from within php.

This is truly a security bug in windows or php itself since it should
properly map directories to the posix way. In any case, I don't really
care if it works or does not work on windows. Let them pay for their
software and security; they aren't getting it for free from me.

> >
> >$file = (strstr($file_get, '../') === true) ? '' : $file_get; // protect
> >from site traversing
> >
> >Do let us know when this has been fixed.
> >
> >Thanks.
>

-- 
Seth Alan Woolley [seth at positivism.org], SPAM/UCE is unauthorized
Quality Assurance Team Leader & Security Team: Source Mage GNU/linux
Linux so advanced, it may as well be magic http://www.sourcemage.org
Secretary Pacific Green Party of Oregon http://www.pacificgreens.org
Key id 00BA3AF3 = 8BE0 A72E A47E A92A 0737  F2FF 7A3F 6D3C 00BA 3AF3

_______________________________________________
Full-Disclosure - We believe in it.
Charter: http://lists.grok.org.uk/full-disclosure-charter.html
Hosted and sponsored by Secunia - http://secunia.com/

• application/pgp-signature attachment: stored

• Previous message: Rob Holland: "[Full-disclosure] pam_ldap/nss_ldap password leak in a master+slave+start_tls LDAP setup"
• Next in thread: Kaf Oseo: "[Full-disclosure] Re: Directory traversal in source.php not fixed."
• Reply: Kaf Oseo: "[Full-disclosure] Re: Directory traversal in source.php not fixed."
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ] [ attachment ]