-
An error in several GodeGrrl PHP scripts has been found that can potentially let a malicious web user read any file on your website, upload their own files, or otherwise cause mischief and mayhem. The vulnerable scripts are:
PHPCurrently version 2.0 and prior
PHPQuotes version 1.0 and prior
PHPCalendar version 1.0 and prior
PHPClique version 1.0 and prior
PHPFanBase version 2.1 and prior
Input passed to the "siteurl" parameter in "protection.php" isn't properly verified, before it is used to include files. This can be exploited to include arbitrary files from external and local resources.
REMEDIES:
If you have the ability to edit the php.ini file (PHP configuration file) on your webserver, set the "register_globals" variable to "off".*
If you do not have the ability to edit the configuration file, consider using an alternate script system until a new version is provided that fixes the vulnerability.
OR, if you are an uber PHP guru, you can edit protection.php yourself to make sure the siteurl variable has not been tainted.
References:
http://secunia.com/advisories/17542/ (http://\"http://secunia.com/advisories/17542/\")
http://www.frsirt.com/english/advisories/2005/2402 (http://\"http://www.frsirt.com/english/advisories/2005/2402\")
*NOTE: I do not know if setting "register_globals" to "off" will break any of the scripts.
EDIT: That will teach me to multi-task while trying to process someone else's code. I incorrectly diagnosed the problem from the security advisories above. Text of post changed to reflect actual threat. Thread opened for discussion and sharing of suggestions of how to ensure the siteurl variable is not tainted.
-
*bump*
important changes to post.
-
Thank you, Johan! *has quickly converted the last phpfanbase fanlisting to Enth on her server* @_@
-
Thanks for opening up the thread.
I have a question on this issue. I don't use Fanbase, but my two hostees do. Now, the issue is with the part of the protection.php?
// This is the page to show when the user has been logged out
$logout_page = "$siteurl";
-
I know other CodeGrrl scripts, as they are now, assume register_globals is turned on, so it's likely that these will break when it's turned off. If so, they can probably be fixed by adding this somewhere:
extract($_GET, EXTR_SKIP);
extract($_POST, EXTR_SKIP);
I'm told that doesn't compromise security, but Johan can probably say whether or not it really does better than I can. ^^
-
I have a question....before when I used phpfanbase I had to add the
php_flag register_globals on
to my .htaccess to get the script to work. Now that I use Enthusiast, should I remove this snippet from my .htaccess?
-
[quote name='Shell' date='Nov 14 2005, 08:40 PM']I have a question....before when I used phpfanbase I had to add the
php_flag register_globals on
to my .htaccess to get the script to work. Now that I use Enthusiast, should I remove this snippet from my .htaccess?
[post=\"115790\"]<{POST_SNAPBACK}>[/post]
[/quote]
Yeah, remove it. Enthusiast doesn't depend on register_globals, and it's safer to have it off. ^_^
-
that's what i figured....it just never occured to me b/c I simply forgot about it *L*
*goes to fix 22 fls now ^_^*
-
[quote name='Firefly' date='Nov 14 2005, 05:51 PM']Thanks for opening up the thread.
I have a question on this issue. I don't use Fanbase, but my two hostees do. Now, the issue is with the part of the protection.php?
// This is the page to show when the user has been logged out
$logout_page = "$siteurl";[post=\"115777\"]<{POST_SNAPBACK}>[/post]
[/quote]
Yes, that is the problem code there.
-
[quote name='Mura' date='Nov 14 2005, 05:54 PM']I know other CodeGrrl scripts, as they are now, assume register_globals is turned on, so it's likely that these will break when it's turned off. If so, they can probably be fixed by adding this somewhere:
extract($_GET, EXTR_SKIP);
extract($_POST, EXTR_SKIP);I'm told that doesn't compromise security, but Johan can probably say whether or not it really does better than I can. ^^
[post=\"115778\"]<{POST_SNAPBACK}>[/post]
[/quote]
According to the PHP online manual (http://\"http://www.php.net\"), extract (http://\"http://us3.php.net/extract\") breaks an array into individual variables. The "EXTR_SKIP" flag prevents PHP from clobbering existing variables with the extracted data:
Do not use extract() on untrusted data, like user-input ($_GET, ...). If you do, for example, if you want to run old code that relies on register_globals temporarily, make sure you use one of the non-overwriting extract_type values such as EXTR_SKIP and be aware that you should extract in the same order that's defined in variables_order within the php.ini.Basically what it's saying is when you use it on $_GET and $_POST, ALWAYS add the EXTR_SKIP flag to make sure the get and post data doesn't overwrite your existing variables. Of course not using "register_globals" is encouraged by the PHP folks, but this appears to be a decent fix.
-
(whee, 3 in a row)
Just to be clear, I want to state that in an effort to remain impartial to popular management systems, I am not naming alternatives, nor am I badmouthing CodeGrrl's scripts. I am just alerting potential users of those scripts of vulnerabilities that have been found and documented.
-
Thanks. Since I use Enthusiast though, I can't turn the register globals off. =.= So I'm making a quckie script for my hostees to use.
-
[quote name='Firefly' date='Nov 15 2005, 07:48 PM']Thanks. Since I use Enthusiast though, I can't turn the register globals off. =.= So I'm making a quckie script for my hostees to use.
[post=\"116059\"]<{POST_SNAPBACK}>[/post]
[/quote]
...Yes you can. Enthusiast doesn't require register_globals.
-
Er, rather, they are off. Blah, just ignore me.
-
good thing i'm changing to enth3 o.o
-
There's a post about the discovered security flaw at CG's forum right here (http://\"http://www.codegrrl.com/forums/index.php?showtopic=11116\"), which was posted just a few hours ago. Apparently you just have to replace protection.php. :3
*goes to do that for her fls that aren't moved yet* I guess it was a good thing I decided to go with Enth when I started moving instead of sticking with fanbase then. ^^; Hmmm...the word "hack/hackers/hacking" seems to be a regular in my vocabulary this week. xD;;
-
Yes that fix will work.
-
Just to reinforce the importance of fixing stuff like this (hopefully she won't mind my mentioning it), our own Loki just had her domain nuked by this very vulnerability, within hours of the announcement. It happens, people. Take care. ^_^
-
Thank you so much for this information, all my fanlistings are still running on Fanbase because I haven't had the time to switch them all over to Enth. ^_^ It's good to know that there's an easy way to fix the problem, for now at least. ^^;
-
Thanks for posting this. I fixed my two hostees script, so hopefully everything will be good for the time period.
-
Thank you so much for posting about this. Some of my fanlistings are still running on PHPFanbase and I have been hesitant about changing all of my fanlistings to Enthusiast. But this will make me to give a good second thought about it.
-
Thanks very much for the information Johan! I've updated my (online) fanlistings with the fixed protection.php file... ^_^
Maybe the best solution is change all the fanlistings to Enthusiast, but maybe I'll do in the future... xD
-
thank you for the information~! ^^
*sweats* I'm glad they fixed the problem... I'm really just too lazy to switch to enthusiast...
-
Hmmm... Sasha has taken the scripts offline (http://\"http://www.codegrrl.com/scripts/\") due to other security risks. For the moment at least.
Anyway, I suppose this sort of begs the question... does anyone (glances in Johan's general direction :kitty: ) know about security risks in the method CodeGrrl used for dynamic includes?
<?php
include('header.inc');
include 'config.php'; // you can include the config file here once
if(!$_SERVER['QUERY_STRING']) {
?>
CONTENT
<?php
} elseif ($_SERVER['QUERY_STRING'] == "whatever") {
?>
CONTENT
<?php
} include('footer.inc');
?>
Could that be used to call external documents?
And for the record (just in case), my intention isn't to attack CodeGrrl... I love the site and the tutorials. I think we all understand that sometimes things like this just happen.
-
Oh, dear, I hope not. :(
[Edit] I checked with Code Grrl, and this is the response I got:
If you are referring to NL-ConvertToPHP, don't worry, it is safe. :kitty:
Or do you mean this tutorial? *goes to look at code*
ETA: The tutorial looks ok, too. It checks the page to make sure it exists before including it.
-
[quote name='Mura' date='Nov 16 2005, 08:25 PM']Anyway, I suppose this sort of begs the question... does anyone (glances in Johan's general direction :kitty: ) know about security risks in the method CodeGrrl used for dynamic includes?
<?php
include('header.inc');
include 'config.php'; // you can include the config file here once
if(!$_SERVER['QUERY_STRING']) {
?>
CONTENT
<?php
} elseif ($_SERVER['QUERY_STRING'] == "whatever") {
?>
CONTENT
<?php
} include('footer.inc');
?>
Could that be used to call external documents?[/quote]
Using the $_SERVER['QUERY_STRING'] method, at first glance, appears to be fine. (PHP.net documentation of pre-defined variables, including the $_SERVER array (http://\"http://us2.php.net/reserved.variables\")). The important bit is what the programmer does with that information. It is up to the programmer to, for example, check the 'QUERY_STRING' variable and make sure (1) the file exists, (2) the file is in the proper context (i.e. local vs. remote, basically not something the web server should not be accessing). THAT is where 99% of all vulnerabilities come from - programmers who do not check data the program receives. Never, ever trust your users to provide the right data. 99% of them will, but there will be that 1% who will input incorrect data because (1) they made a mistake, or (2) they're trying to break your program.
Hmmm... Sasha has taken the scripts offline (http://\"http://www.codegrrl.com/scripts/\") due to other security risks. For the moment at least.
....
And for the record (just in case), my intention isn't to attack CodeGrrl... I love the site and the tutorials. I think we all understand that sometimes things like this just happen.
[post=\"116422\"]<{POST_SNAPBACK}>[/post]
I am encouraged that they took the scripts off-line while they fix them. Though to be honest I am troubled by the statement "We are unsure at the moment where these issues lie and how to fix them[...]". I also do not intend to attack CodeGrrl. Tutorials and open source are a wonderful way for people to learn new methods, techniques, tools, especially since php.net is better as a dictionary and not very well suited to learning the language. However it is starting to appear to me (and this is all my opinion) that the programmers there haven't quite grasped the full understanding of how powerful PHP is, and why any implementation needs to be locked down tighter than a snare drum.
-
Using the $_SERVER['QUERY_STRING']method, at first glance, appears to be fine. (PHP.net documentation of pre-defined variables, including the $_SERVER array). The important bit is what the programmer does with that information. It is up to the programmer to, for example, check the 'QUERY_STRING' variable and make sure (1) the file exists, (2) the file is in the proper context (i.e. local vs. remote, basically not something the web server should not be accessing). THAT is where 99% of all vulnerabilities come from - programmers who do not check data the program receives. Never, ever trust your users to provide the right data. 99% of them will, but there will be that 1% who will input incorrect data because (1) they made a mistake, or (2) they're trying to break your program.
So basically you're basically saying it would be best to provide additional conditionals to handle files that are not there and files that are not on the server, right?
-
[quote name='Mura' date='Nov 18 2005, 11:00 AM']So basically you're basically saying it would be best to provide additional conditionals to handle files that are not there and files that are not on the server, right?
[post=\"116747\"]<{POST_SNAPBACK}>[/post]
[/quote]
The basic idea is to examine the data before you use it. In the case of the bit of code we're talking about, the proper solution would be to look at the QUERY_STRING variable, determine what file it's wanting to load, and have PHP check to see if the file exists and should be loaded here before trying to load it. Additional conditionals are also helpful, espcially for development and debugging.
-
Thank you so much for posting this information before anything drastic could've happened! I would love to switch to Enthusiast but it's confusing to me for some reason and I love my Fanbase! *hugs it* I've replaced all my protection.php files. Thank you again!
-
Thanks for posting it, I've also edited the protection.php files yesterday (And all my hostees files too!) :kitty:
-
[quote name='Daisy' date='Nov 18 2005, 02:39 PM']Thank you so much for posting this information before anything drastic could've happened! I would love to switch to Enthusiast but it's confusing to me for some reason and I love my Fanbase! *hugs it* I've replaced all my protection.php files. Thank you again!
[post=\"116783\"]<{POST_SNAPBACK}>[/post]
[/quote]
Hmm... the CodeGrrl staff say that there are additional security flaws that they haven't fixed. So the edited script won't necesarily be enough. Just so you know.
-
[quote name='Mura' date='Nov 18 2005, 06:40 PM'][quote name='Daisy' date='Nov 18 2005, 02:39 PM']Thank you so much for posting this information before anything drastic could've happened! I would love to switch to Enthusiast but it's confusing to me for some reason and I love my Fanbase! *hugs it* I've replaced all my protection.php files. Thank you again!
[post=\"116783\"]<{POST_SNAPBACK}>[/post]
[/quote]
Hmm... the CodeGrrl staff say that there are additional security flaws that they haven't fixed. So the edited script won't necesarily be enough. Just so you know.
[post=\"116829\"]<{POST_SNAPBACK}>[/post]
[/quote]
Which is why I made up my own script for my two hostees. I rather not take the risk. =.=
-
Just thought I'd add that the developer of FanAdmin has said that it is safe for FanAdmin users to delete admin.php, login.php, invalidlogin.php, and protection.php files from their fanlistings (since you do all the admin stuff through FanAdmin) :kitty:
-
[quote name='Annie' date='Nov 19 2005, 08:42 AM']Just thought I'd add that the developer of FanAdmin has said that it is safe for FanAdmin users to delete admin.php, login.php, invalidlogin.php, and protection.php files from their fanlistings (since you do all the admin stuff through FanAdmin) :kitty:
[post=\"116946\"]<{POST_SNAPBACK}>[/post]
[/quote]
But what if the hacker attacks the FanAdmin files instead? Is there a way to prevent that?
-
Phew. When I heard about hackers trying to hack through Codegrrl-powered sites, I immediately edited the protection.php from all of my fanlistings.
-
[quote name='Annie' date='Nov 19 2005, 01:42 AM']Just thought I'd add that the developer of FanAdmin has said that it is safe for FanAdmin users to delete admin.php, login.php, invalidlogin.php, and protection.php files from their fanlistings (since you do all the admin stuff through FanAdmin) :(
[post=\"116946\"]<{POST_SNAPBACK}>[/post]
[/quote]
One host pinpointed FanAdmin as also vulnerable -- at least according to this post (http://\"http://www.codegrrl.com/forums/index.php?showtopic=11116&view=findpost&p=55561\") from a CodeGrrl member :kitty:
-
[quote name='kirisame' date='Nov 19 2005, 08:51 AM']But what if the hacker attacks the FanAdmin files instead? Is there a way to prevent that?
[post=\"117022\"]<{POST_SNAPBACK}>[/post]
[/quote]
100% security is 100% Unobtanium - it's a myth, can't happen, no such thing.
That being said, once that is understood, should people live in fear every day? No. Instead, keep tabs on the person providing your code; make sure you're running the latest version. Maybe this is a good opportunity to grab a book on PHP and try to learn it, though making code bulletproof requires a lot of experience and knowledge.
As far as preventing exploits on FanAdmin, specifically, that would require an audit of the code to see what it does, how it does it, and why. A non-specific thing that you can check is to make sure everything is not world-writable, unless it is absolutely necessary for a script to function - and in that case modify the permissions on only the files and directories necessary.
[quote name='Angela' date='Nov 20 2005, 05:02 PM']One host pinpointed FanAdmin as also vulnerable -- at least according to this post (http://\"http://www.codegrrl.com/forums/index.php?showtopic=11116&view=findpost&p=55561\") from a CodeGrrl member :kitty:
[post=\"117305\"]<{POST_SNAPBACK}>[/post]
[/quote]
*shudder*
pwned. That person needs to change all her passwords and account information NOW.