Ravens PHP Scripts: Forums
 

 

View next topic
View previous topic
Post new topic   Reply to topic    Ravens PHP Scripts And Web Hosting Forum Index -> Security - PHP Nuke
Author Message
Gremmie
Former Moderator in Good Standing


Joined: Apr 06, 2006
Posts: 2415
Location: Iowa, USA

PostPosted: Sat Nov 11, 2006 3:10 pm Reply with quote

Reference: Only registered users can see links on this board! Get registered or login!

(As an aside, after I downloaded this, I noticed it already was included with Nuke; at least in the 7.9 w/3.2 patches distro. It's in includes, but as far as I can tell, no one is using it... Confused )

So I'm trying this thing out.

Right off the bat, I see two problems with this member function:

Code:


   function escapeString($string, &$connection) {
      // depreciated function
      if (version_compare(phpversion(),"4.3.0", "<")) mysql_escape_string($string);
      // current function
      else mysql_real_escape_string($string);
      return $string;
   }


1) It is not capturing the return values of the escape_string functions; instead it is just returning the original string. Ummmm...

2) Although it makes you pass in a reference to a database resource, it doesn't use it in the mysql_real_escape_string() call. This is still okay, I guess, cause according to the docs, this function will use the last connection returned from a mysql_connect() call if you don't supply one yourself.

I don't think the safeSQL() function (that eventually calls the above function) must have been tested much. The author doesn't seem to answer emails either.

Once I fixed those up, I noticed one annoying thing. Newlines ("\n") and returns ("\r") are converted to the character sequences \n and \r, so they will look pretty in a log file (I guess). The docs for MySQL say you don't really need to escape these to use them in a query. As a consequence, I guess I am going to have to str_replace('\n', "\n", $dbVariable); when I read it back out if I want to preserve those. Confused

So now I'm wondering out loud....should I just use addslashes() like everyone else? How many people run Nuke on non-MySQL servers anyway? I realize its safer to use mysql_real_escape_string, since its supposed to take into account character sets.

Another random question. The InputFilter class passes the resource connection by reference. Is this necessary? Doesn't PHP know its a resource and does the right (most efficient) thing?

Thanks again.
 
View user's profile Send private message
Gremmie
PostPosted: Sun Nov 12, 2006 10:56 am Reply with quote

I also changed this:
Code:


   function remove($source) {
      $loopCounter=0;
      // provides nested-tag protection
      while($source != $this->filterTags($source)) {
         $source = $this->filterTags($source);
         $loopCounter++;
      }
      return $source;
   }   


To this:
Code:


   function remove($source) {
      // provides nested-tag protection
      while ($source != ($str = $this->filterTags($source)))
      {
         $source = $str;
      }
      return $source;
   } 


It will (often) cause one less call to $this->filterTags() compared to the original.

I've decided to try and use this thing, but as I said in my first post, the safeSQL() function looks to me to be kind of broken. I will post a modified version after I'm done tweaking. I've decided to do the nl2br() thing followed by a removal of all "\n" and "\r" before saving to the database.
 
evaders99
Former Moderator in Good Standing


Joined: Apr 30, 2004
Posts: 3221

PostPosted: Sun Nov 12, 2006 3:13 pm Reply with quote

Hmm interesting. I didn't know chatserv was using this class. It has a lot of potential, it looks like Mambo is using it too.

Currently it doesn't seem like it is being used in 7.9 Patched yet.

You are right about the escapeString function

Whether mysql_real_escape_string is better, I have no idea. But since the old mysql_escape_string is being deprecated, might as well use the "real" verison

_________________
- Only registered users can see links on this board! Get registered or login! -

Need help? Only registered users can see links on this board! Get registered or login! 
View user's profile Send private message Visit poster's website
Gremmie
PostPosted: Sun Nov 12, 2006 3:38 pm Reply with quote

Thanks for looking it over Evaders.

The only caveat about using mysql_real_escape_string is that it is nailing us to MySQL. Does anyone run Nuke on other databases? The database abstraction layer (that came from phpBB, right???) is set up to handle many database backends, but I've often wondered if anyone really uses it on anything but MySQL. I also wonder if there are certain queries in all the code that are MySQL specific.
 
kguske
Site Admin


Joined: Jun 04, 2004
Posts: 6383

PostPosted: Sun Nov 12, 2006 3:59 pm Reply with quote

There has been a great deal of discussion regarding this as a whol, but maybe the question is whether or not you want to support your module running on databases other than MySQL?

There are a few mods that use MySQL-specific SQL.

_________________
I google, therefore I exist...
Only registered users can see links on this board! Get registered or login!
 
View user's profile Send private message
Gremmie
PostPosted: Sun Nov 12, 2006 5:30 pm Reply with quote

Well, if in fact 90% of the world runs Nuke on MySQL, then I would sleep a lot better using mysql_real_escape_string(). Does anyone have a feel for that? Do people use Nuke on other databases?
 
kguske
PostPosted: Sun Nov 12, 2006 5:32 pm Reply with quote

I'd say it's probably higher than 90%...
 
evaders99
PostPosted: Sun Nov 12, 2006 9:02 pm Reply with quote

While this is an unofficial survey - here are the results from the surveys I put up

Most people use MySQL Only registered users can see links on this board! Get registered or login!

But there is a split in whether we should dump the support for other databases Only registered users can see links on this board! Get registered or login!
 
Gremmie
PostPosted: Sun Nov 12, 2006 9:15 pm Reply with quote

Well it would sure help if someone were to modify the database abstraction layer to add an escapeString() type member function. The MySQL version could implement it with mysql_real_escape_string(), and the other versions could do whatever they have. And if they don't have one, it could fall back to addslashes().
 
kguske
PostPosted: Sun Nov 12, 2006 10:28 pm Reply with quote

I thought there might be other surveys, but hadn't seen that. Thanks!
 
technocrat
Life Cycles Becoming CPU Cycles


Joined: Jul 07, 2005
Posts: 511

PostPosted: Mon Nov 13, 2006 1:15 pm Reply with quote

We have been using this in Evo for some time now, it works pretty well, though it can be a pain in some instances since it tends to be a bit heavy handed at times. I would suggest not allowing it to scan if you are in the admin area.

Also take the Mambo version of this filter, they fixed a ton of little bugs with it. Some of which I submitted to the original author and never heard anything back.

Finally, this should help you with your strip problem:
Code:
        static $magic_quotes;

        if (!isset($magic_quotes)) $magic_quotes = get_magic_quotes_gpc();
        if ($magic_quotes) $string = stripslashes($string);
        return (version_compare(phpversion(), '4.3.0', '>=')) ? mysql_real_escape_string($string) : mysql_escape_string($string);

_________________
Only registered users can see links on this board! Get registered or login!
Only registered users can see links on this board! Get registered or login! / Only registered users can see links on this board! Get registered or login! 
View user's profile Send private message
Gremmie
PostPosted: Mon Nov 13, 2006 1:38 pm Reply with quote

Thanks technocrat! I thought I read you were using it in Evo, and I downloaded Evo over the weekend to check it out, but I haven't looked yet. Are the bug fixes you mention for it in the Evo distro?

I also tried to contact the author, but my email came back. I don't think rootcube.com is a valid domain anymore.
 
technocrat
PostPosted: Mon Nov 13, 2006 1:49 pm Reply with quote

If you have v2, they are, though I would not use the Evo version because we used cache and debugging, which will toss any other nuke.

I do not think so either. Too bad as it was a good project, just needed a few fixes here and there.
 
kguske
PostPosted: Mon Nov 13, 2006 8:27 pm Reply with quote

technocrat, since I first learned of Only registered users can see links on this board! Get registered or login! from your post, I'm curious to know your thoughts on it. Why did you select Cyberai for Nuke Evo?
 
technocrat
PostPosted: Tue Nov 14, 2006 9:04 am Reply with quote

Well at the time, purifier was pretty new. Plus it did not have the ability to fix tags, which at the time I thought was a good feature. Also the coding and project looked better to me.

Now that time has passed I feel that Cyberai is good, and has a Mambo backing, but the project on it's own looks to be dead. Which is a shame.

So if I was going to do it all over again, I would probably look more closely at the other choices out there, and see how well they worked before making any choices. Especially since purifier has grown into a very well thought out project.
 
Gremmie
PostPosted: Tue Nov 14, 2006 11:55 am Reply with quote

The Cyberai InputFilter also seems to be in Joomla. I have not had a chance to download Mambo and do comparisons yet. I did take a quick glance at the version in Evo, and did see right away many tweaks. Thanks for the heads up with that again.
 
technocrat
PostPosted: Tue Nov 14, 2006 12:03 pm Reply with quote

Joomla and Mambo are done by the same group if I remember correctly.

Again, do not forget to turn off the filter for admins. If you look at the code in the mainfile and modules in Evo, you will see what we did to fix some of the problems we ran across in the year and a half we been using it. You should be able to apply it to any other nuke system.
 
Gremmie
PostPosted: Tue Nov 14, 2006 12:16 pm Reply with quote

I thought Joomla split from Mambo...

Anyway...what kind of problems did you run into with that class for admins?
 
technocrat
PostPosted: Tue Nov 14, 2006 12:21 pm Reply with quote

How do you think the filter will react if you want to add javascript to a block, through the block admin, or an embeded object like music or a video. Wink
 
Gremmie
PostPosted: Tue Nov 14, 2006 12:52 pm Reply with quote

Ah, I see...I always use file blocks for that kind of stuff. But I am going to write an admin section for my module, so I will keep that in mind.

Do you assume in Evo that the database is trusted? Do you filter strings you read from the database with the InputFilter class? Or do you assume the admin knows what they are doing and that only filtered stuff got put into the database?
 
technocrat
PostPosted: Tue Nov 14, 2006 1:07 pm Reply with quote

Do you assume in Evo that the database is trusted? -> Yes
Do you filter strings you read from the database with the InputFilter class? -> No

I figure there is no point in going after code that is already in the DB as it is probably to late. If your an admin I would assume you have a reason to put that kind of code in yourself. If it's a hacker in your admin area then you have worse problems then try to get html filtered out.
 
evaders99
PostPosted: Tue Nov 14, 2006 2:39 pm Reply with quote

PHP "security by layers" approach would say otherwise, treat the database itself as unclean source of data, as code may have been injected into your database that will ouput insecure code....

I believe that's the approach that chatserv has taken with the Patched files, but I don't think Raven agrees here.
 
technocrat
PostPosted: Tue Nov 14, 2006 2:50 pm Reply with quote

I have always gone with garbage in, garbage out. Plus the way I look at it, if something is inserted in your DB then you already have problems that go beyond what the filter can help you with.

Also why take up DB space storing bad data?
 
kguske
PostPosted: Tue Nov 14, 2006 3:57 pm Reply with quote

Garbage in, garbage out.
 
Gremmie
PostPosted: Tue Nov 14, 2006 8:06 pm Reply with quote

It looks like Mambo is using the stock version found on the web. Joomla has modified it in a similar way as Evo, but I didn't do line-by-line diffs to see exactly what was changed between Evo and Joomla.
 
Display posts from previous:       
Post new topic   Reply to topic    Ravens PHP Scripts And Web Hosting Forum Index -> Security - PHP Nuke

View next topic
View previous topic
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
You can attach files in this forum
You can download files in this forum


Powered by phpBB © 2001-2007 phpBB Group
All times are GMT - 6 Hours
 
Forums ©