From: msopacua at idg.nl (Melvyn Sopacua)
Subject: pMachine Cross Site Scripting in Search
  module and Path Disclosures

[ hereby notifying vendor, as advisory doesn't have any indication of that ]

At 15:19 6/19/2003, Lorenzo Hernandez Garcia-Hierro wrote:

>Product: pMachine
>Vendor: pMachine <www.pmachine.com>

[ ... ]

>-----------------------------------------
>SECURITY HOLES FOUND and PROOFS OF CONCEPT:
>-----------------------------------------
>
>I encountered few security holes related with path disclosures  and Cross
>Site Scripting vulnerabilities in Search module.
>
>---------------------------
>|    PATH DISCLOSURES     |
>---------------------------
>
>There are some path disclosures in some files of pMachine , with this you
>can get the real (local) installation path of
>the pMachine scripts.
>
>Proof of Concepts:
>
>http://[TARGET]/[pMachine PATH]/index.php?sfx=./nothing
>http://[TARGET]/[pMachine PATH]/inc.lib.php?sfx=./nothing
>http://[TARGET]/[pMachine PATH]/inc.cp.php?sfx=./nothing
>http://[TARGET]/[pMachine PATH]/lib/weblog.add.php <-- access directly
>http://[TARGET]/[pMachine PATH]/lib/comment.add.php <-- access directly
>
>Possible other files using this variable (sfx) are vulnerable.

Look - I don't know what crappy php setup you use to perform your tests, but I
just downloaded the 2.2.1 version (listed as vulnerable by you) and installed
it.
The /pm/index.php?sfx=./nothing shows me exactly that - nothing, because I have
display_errors Off by default in production.
Again - pMachine does nothing that enables this:
$ find . -type f -print | xargs grep display_errors |wc -l
0

Then - this IS a very dangerous package, but you have done very little
research to warrant an advisory. It is dangerous because it circumvents
register_globals explicetely, which is why $sfx can be set from a url
in the first place. The offending code is in inc.cp.inc:
//      ----------------------------------------------
//              REGISTER GLOBALS OFF
//      ----------------------------------------------

if (isset($HTTP_COOKIE_VARS))
{
     while(list($var,$val)=each($HTTP_COOKIE_VARS))
     {
         $$var=$val;
     }
}
if (isset($HTTP_GET_VARS))
{
     while(list($var,$val)=each($HTTP_GET_VARS))
     {
         $$var=$val;
     }
}
if (isset($HTTP_POST_VARS))
{
     while(list($var,$val)=each($HTTP_POST_VARS))
     {
         $$var=$val;
     }
}
if (isset($HTTP_SERVER_VARS))
{
     while(list($var,$val)=each($HTTP_SERVER_VARS))
     {
         $$var=$val;
     }
}
// END

For instance:
If use_session_id=1 (which is not the default), one needs just one
session being active to be able to log in, when magic_quotes_gpc is
off, due to an SQL injection:

/pm/index.php?sx=foo'%20OR%20session_id=session_id%20AND%20'a'='a

Creates:
select count(*) as count from pmachine__session where session_id = 'foo' OR 
session_id=session_id AND 'a'='a'
at index.php line 261.

As $sx is not validated correctly:
     // completely redundant code here, cause it's already done in
     // afore mentioned register_globals workaround.
     if (isset($HTTP_GET_VARS['sx']))
     {
         $sx = ($HTTP_GET_VARS['sx']);
     }
     else
     {
         $sx = "";
     }

     if ($sx != "")
     {
         $query = new DB_query($db, "select count(*) as count from 
$db_session where session_id = '$sx'");

Depending on the first person to come up in the result set you can gain 
priveleges
from a normal user, to the admin user.

This would have been much more difficult if the check:
if($result > 0)

would be:
if($result == 1)

because then only one row qualifies, which in practice would mean, that the
user needs a valid member_id to do anything usefull.

But of course, $sx should be addslashed().

And the "let's just ignore register_globals" approach is really not very good
coding practice.

>Lorenzo Hernandez Garcia-Hierro
>  --- Computer Security Analyzer ---
>  --Nova Projects Professional Coding--

This is actually ment to help you:
1) setup a good environment (ie: use php.ini-recommended and not php.ini-dist)
2) Test the package thoroughly - these are minor issues you mention while you
    left a pretty easy (took me 10 mins to find and I ain't even a "security
    analyzer") and authentication comprising error untouched.
3) Quality over quantity:
    You posted 3 advisories in 2 days:
    One about a package that shouldn't be in public view

    One about an example script (I'm surprised you didn't bitch about 
apache's printenv
    in /cgi-bin/ yet) - also - I'm not sure this is a crash related to perl 
or Sambar,
    as I have done a fair ammount of beta testing on Sambar in earlier 
days. Your code
    example also doesn't show that, because tr/+/ /  should only crash when 
perl screws
    up or you have very little memory. Since search.pl uses the internal 
search engine
    shipped with Sambar IIRC, you could have tested the same query using an 
'stm'
    approach and it would basically mean, that it cannot handle more than x 
keywords.
    If you have done that, then you should mention that.

    One about minor issues, when big ones are left uncovered.

Now - I think you would serve the weblogging community a big favour, when 
you do
a thorough overhaul of the package (ie: finish what you started), cause 
frankly - I
haven't the time and don't blog.


Met vriendelijke groeten / With kind regards,

Webmaster IDG.nl
Melvyn Sopacua

"Freedom includes the freedom to disagree with me
and still use my software." - Arnt Gulbrandsen,
Author of Leafnode.

Powered by blists - more mailing lists