Cyberai InputFilter class

Posted: Sat Nov 11, 2006 3:10 pm

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.

Posted: Sun Nov 12, 2006 10:56 am

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.

Posted: Sun Nov 12, 2006 3:13 pm

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

Posted: Sun Nov 12, 2006 3:38 pm

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.

Site Admin Joined: Jun 04, 2004 Posts: 6432

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.

Posted: Sun Nov 12, 2006 5:30 pm

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?

Posted: Sun Nov 12, 2006 5:32 pm

I'd say it's probably higher than 90%...

Posted: Sun Nov 12, 2006 9:02 pm

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! ]

Posted: Sun Nov 12, 2006 9:15 pm

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().

Posted: Sun Nov 12, 2006 10:28 pm

I thought there might be other surveys, but hadn't seen that. Thanks!

Posted: Mon Nov 13, 2006 1:15 pm

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);

Posted: Mon Nov 13, 2006 1:38 pm

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.

Posted: Mon Nov 13, 2006 1:49 pm

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.

Posted: Mon Nov 13, 2006 8:27 pm

technocrat, since I first learned of HTML Purifier from your post, I'm curious to know your thoughts on it. Why did you select Cyberai for Nuke Evo?

Posted: Tue Nov 14, 2006 9:04 am

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.

Posted: Tue Nov 14, 2006 11:55 am

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.

Posted: Tue Nov 14, 2006 12:03 pm

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.

Posted: Tue Nov 14, 2006 12:16 pm

I thought Joomla split from Mambo...

Anyway...what kind of problems did you run into with that class for admins?

Posted: Tue Nov 14, 2006 12:21 pm

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

Posted: Tue Nov 14, 2006 12:52 pm

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?

Posted: Tue Nov 14, 2006 1:07 pm

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.

Posted: Tue Nov 14, 2006 2:39 pm

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.

Posted: Tue Nov 14, 2006 2:50 pm

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?

Posted: Tue Nov 14, 2006 3:57 pm

Garbage in, garbage out.

Posted: Tue Nov 14, 2006 8:06 pm

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.