Fractured Filterin - Revisted

Posted: Tue Aug 07, 2007 8:23 am

Great idea you have here M. Smile

I just went over and downloaded Nuke Evo. It has the YA module you referred to plus a whole lot more. Why we are duplicating (overlapping) their efforts is a question for me ... but another thread I suppose. For instance, they have a caching system which uses classes (sort of like we are talking about in other threads here) and SQL error capture (which I've been promoting in yet other threads) and a whole lot more. It ain't like we are rolling in programming resources to the point where we can afford to duplicate efforts.

Says he naively. I will look more specifically at the YA module there and report back.

registered · Posted: Tue Aug 07, 2007 9:13 am

Montego is correct, that is what we did. We made a number of improvements to it. However in all honesty we have had discussions about just ripping it out and making a new one. We rewrote much of the code but its still kind of a mess because we tried to keep the core of YA when we really shouldn't have.

Fkelly, I have said over and over again that you guys should use the cache and debugging code that we wrote. Not sure why no one took a look at it before now.

I would also highly suggest using the new variable class I wrote. You can see some of it 2.0.5 but it adds more protection in the release we are working on now. I would be happy to pass that along as well.

Posted: Tue Aug 07, 2007 10:55 am

First, Technocrat, thanks for the feedback and info on YA. I really don't want us going down any dead ends and it is likely we would arrive at the same conclusions you have after expending a lot of work. I am sure we will keep your feedback in mind.

Quote:

Fkelly, I have said over and over again that you guys should use the cache and debugging code that we wrote. Not sure why no one took a look at it before now.

I guess it's better late than never. I knew you did something with logging SQL errors and I've posted a small hack to do the same in RN (and use it on my own web site) but I will look at what you did. I've already looked around the cache classes a bit. Gremmie is our "class" guy and I'm sure he'd be interested too. Or I'd guess I should say.

And Gremmie has been working on some classes for RN and I'm sure we are both interested in what you are doing for a variable class.

registered · Posted: Tue Aug 07, 2007 6:25 pm

On the contrary, many say I have no class.... Laughing

YA blows big time. We need something better. I have downloaded and looked at Evo before. The Evo guys and Technocrat seem very capable and they are onto something so I would welcome collaboration. Whether we share/use what they already have or work on a new replacement with them would be fine by me.

technocrat, what is the purpose of the variable class?

I've been exploring some PHP5 classes at home for validating forms, displaying forms, doing modules, displaying web pages, etc.

BTW, I just got the book "PHP 5 Objects, Patterns, and Practice" by Zandstra today. Cool

Posted: Wed Aug 08, 2007 9:06 am

To safely retrieve passed in info from $_GET, $_POST, $_REQUEST, etc.

Here is an example:
Lets say we expect a POST back with a user_id, name, and email_address.

Code:

global $_GETVAR;


//$user_id will be an int if there is a valid number in $_POST['user_id']


//If there isn't $user_id will be null


$user_id = $_GETVAR->get('user_id', 'POST', 'int');


//Get the string name


$name = $_GETVAR->get('name', 'POST');


//Email will only be valid if there is a valid email address.


//This function is hard to fool but its not fool proof


$email_address = $_GETVAR->get('email_address', 'POST', 'email');

This class will strip magic_quotes if on. Then add slashes to everything passed in using mysql_real_escape_string, mysql_escape_string, addslashes in that order depending on if the functions are there or not. It also will deep slash arrays.

You can use regex against a variable and also force what the default return is.

Here is a generic example:

Code:

//$variable = $_GETVAR->get($var, $loc, $type, $default, $minlen, $maxlen, $regex);


$foo = $_GETVAR->get('foo', '_GET', 'string', null, 1, 10, '/^foo/i');

In my opinion its a pretty well done class Wink

Posted: Wed Aug 08, 2007 9:46 am

I agree with you about the class Technocrat. Gremmie has posted (no pun intended) some similar code here. I think it was 64bit who coined the phrase "fractured filtering" and I think it is fractured filtering that we are working towards eliminating. If I am not mistaken the fckeditor code also sanitizes variables before they are even posted from a form, though I'm not sure if that just applies to textareas. Then NS re-sanitizes them in its post and get filter eregi's.

Correct me if I'm wrong but aren't we working towards:

1. a form is prepared and submitted
2. the PHP program that receives and processes the form runs a standard class that validates all variables from that form
3. if they pass the validation some sort of flag is set that tells Sentinel to bypass those
4. Sentinel obeys the flag while continuing to validate older programs that don't use the class
???

And somehow these Forum posts need to get incorporated so you don't get banned anytime you post something that Sentinel objects to.

From a Ravennuke development perspective I think that if we had the class in place and some good instructions for using it (for dummies like me) we could just work thru the system form by form subjecting the form submission data to the class based validation while leaving Sentinel protections in place for those forms we didn't get to by the time of a given "release".

Posted: Wed Aug 08, 2007 10:04 am

We should probably move these posts out of here and stop hijacking this post Smile

My idea on this topic is to stop Sentinel from doing that at all. Then use HtmlPurifier (http://htmlpurifier.org/) to clean everything expect if the user is in the admin area or maybe is an admin. That way all bad code gets striped and any HTML that goes through gets forced into XHTML strict. The only time JS should be allowed is when the admin wants to.

Posted: Wed Aug 08, 2007 11:41 am

I'm also sorry for continuing the hijacking but I have a similar idea. I am creating classes for all the different types of common things you would expect in a form: email, alpha only, numeric only, words, a general regex match, or calling a user supplied callback function to do the validation. You can also set properties like required (yes/no), min length, max length, and an error message if it fails validation. You set all these up and then plug them into a form object. When you call the post() function on the form it walks through all the fields and validates them. You can then ask for the error messages and redisplay the form if needed. If everything worked you get an array of keys => values.

I haven't thought this far ahead yet, but here is where you would apply another filter depending on the destination of the form data: to the screen (htmlentities), to the database (mysql_real_escape_string), etc. A general filter for XSS would be there too.

I like your idea too Technocrat. I think we are all moving towards a more automated way of filtering and processing user input. The more we can automate it, the less chance for module writers to goof up. The way it is done now, if it is done at all, is far too tedious and error prone. And those errors lead to cracks in security.

Posted: Wed Aug 08, 2007 12:01 pm

I thought about doing the validation the same kind of way when I started this. The problem is what about stuff outside of just forms. Take news articles for example. When you open open or go directly to it your looking for the sid. Now lets just say I am an idiot and my news has no protection on it. Just something like:
$sql = 'SELECT * FROM news WHERE sid="'.$sid.'"';
So in theory http://www.yoursite.com/modules.php?name=News&file=articles&sid=Hack could be used for evil. Which isn't much of a stretch in the Nuke World I see it all the time.

But if you did
$sid = $_GETVAR->get('sid'
'GET', //Because it should only be in the URL
'int', //Because it should only be a int
0); //Send back a 0 if invalid
That would stop that issue.

So I tried to make it flexible enough to use outside of just forms.

I will send the most current class to you.

Posted: Wed Aug 08, 2007 12:11 pm

Yes that is an excellent suggestion to make it more general.

registered · Posted: Thu Aug 09, 2007 6:53 am

The above thread came from here:

http://www.ravenphpscripts.com/postp105207.html#105207

Posted: Thu Aug 09, 2007 7:02 am

technocrat wrote:

This class will strip magic_quotes if on. Then add slashes to everything passed in using mysql_real_escape_string, mysql_escape_string, addslashes in that order depending on if the functions are there or not. It also will deep slash arrays.

My concern with this, is what is done with the input is really dependent upon the output. I agree that the following should ALWAYS be done:

1. Check for magic quotes and strip if needed

2. Always perform "type" checks (such as INT) that are appropriate for the field.

3. Always perform XSS screaning / cleansing (if not admin) leaving behind only allowed and valid XHTML.

I personally think that a generic class must stop there. What follows would have to be additional functional tests that are specific to a response variable.

For example, if the response is to be only one of a specific set of values, a db read might have to be done to "test" that. Some of these types of simple tests could still be made "generic" enough and be a Step 4 above, but some will not.

Now we get to the "output". I agree that if the next step is to go directly to the DB, proper escaping must be done to prevent SQL injection. However, that is NOT the case if the output will be directly back to an HTML/XHTML page.

Even if the value is to be outputted back to a page, even here, it matters how. If it is to be a straight echo of the HTML/XHTML, that is one thing. If it is to be placed into an input or textarea field of a form, again, it is another thing (should have either htmlentities or htmlspecialchars applied for some security reasons and for HTML/XHTML compliance reasons).

I have had these thoughts for quite some time and I think we even had another topic under the RavenNuke team internal forums started by Evaders to discuss Filtering as well.

Posted: Thu Aug 09, 2007 9:40 am

My thought process when I did that was if you think about it how much information will come through those and not interact with the DB. Very little. On top of the fact that people building things for nuke will many times make a mistake where it's possible to inject. So it came down to more a security issue vs convenience.

If you really have something that is going just to the screen then go ahead and run a stripSlashes against it. But you will find that it's not very often you will find yourself with a need to do that.

To back that up if you look through phpbb, Mambo, Joomola, etc they all follow the same logic. I am not say that it's right or wrong, but to me it just seems a more secure way to go. Thus the reason I took that path.

Posted: Thu Aug 09, 2007 11:43 am

I see your point. It is just not how I typically do it. I guess it is because I see alot of "Preview" functions where the end-user is asked (or can) first preview what he/she entered and how it will look prior to submitting.

You know, I have always used AddSlashes and not used mysql_escape_string. You would have to use a different unscape function right?

Posted: Thu Aug 09, 2007 11:53 am

Addslashes is pretty much a no-no and should only be used when you are left no other options. Which is rare these days.

Here is one of the many articles on it:
http://shiflett.org/blog/2006/jan/addslashes-versus-mysql-real-escape-string

No you can use stripslahes on it because it just looks for them and takes them out. You just have to be sure that slashes were put in or else it might make your user upset.

Posted: Thu Aug 09, 2007 11:58 am

Yeah, I have his book and the most frustrating thing when reading it is that he makes a statement that even mysql_real_escape_string() is also not perfect but then he does not give a solution. Sad

Well, I guess better is certainly, well, better... lol.

I guess, in reality, the best alternative is prepared statements. Hhhmmm... I wonder if this could somehow be genericized...

Posted: Thu Aug 09, 2007 12:39 pm

stripslashes will work fine on a mysql_real_escape_string()'ed piece of data.

I've been using mysqli and prepared statements do indeed rock! You no longer have to escape the text yourself. Yet another reason to switch to PHP5! No more addslashes or mysql_real_escape_string!

Posted: Thu Aug 09, 2007 1:08 pm

Gremmie wrote:

stripslashes will work fine on a mysql_real_escape_string()'ed piece of data.

But, I am not so sure that is the case if the user had inputted a backslash.

I would prefer to see the default behavior of this class be as what has been described, but then maybe another "flag" can be passed to it if the escaping is not desired. That way the developer can choose the exception when needed.