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
evaders99
Former Moderator in Good Standing


Joined: Apr 30, 2004
Posts: 3221

PostPosted: Sun Sep 24, 2006 7:59 pm Reply with quote

I'll have to take a look at your code sometime. Not a big fan of using classes, adds some overhead that may not be necessary. Nor long function names like $my_nohtmlFilter->nohtmlsafeSQL()
But that's just a stylistic thing.

If we do come up with a filter system, it would be something we would have to actively maintain, as well as give people the option to disable.

_________________
- 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
spottedhog
Regular
Regular


Joined: Jun 02, 2004
Posts: 88

PostPosted: Sun Sep 24, 2006 8:08 pm Reply with quote

well.... I just could not get anything to work with using the one class file but somehow have one function for allowable html tags and one for allow zero tags..... hence I just added the god-awful long naming.... Wink Also, for my own thoughts, there would be no way to confuse what goes where... Smile

If a coder could make it so the tag array could be specified for allowed html tags and zero tags, then it could get changed in a heartbeat..... lol.

Thanks for taking a look..... The concept, in my small mind, has some merit.....

The code for InputFilter I posted is pretty much as the original. In the allowed html tags, I put in the tags from the $AllowableHTML. The only minor change is at the end of the file in the escaping code. The code would knock off the leading zero, so I put a snipet in for keeping the leading zero.
 
View user's profile Send private message Visit poster's website
montego
Site Admin


Joined: Aug 29, 2004
Posts: 9456
Location: Arizona

PostPosted: Mon Sep 25, 2006 6:49 am Reply with quote

Quote:

For me, the only issue is on whether to let Admins to use whatever words they want in their inputs, or to check_words for all text inputs. I decided Admins can make the choice, however, I am checking all words from User inputs.


My contention with this statement is that as an admin, if I want to create an HTML block, for example, which has javascript code embedded in it, why make me have to use phpMyAdmin to add that? For me, I cannot access my host admin environment from work... they block all non-standard ports. Therefore, if I have to fix something or want to post something that I should be able to post as an admin (but wouldn't want any non-admin to do this), I'd be screwed.

My issue has nothing to do with "word checking", but with being able to use the system and module administration features un-abated by the stripping of HTML. In the nuke patches now, even data coming out from the db is run through check_html, which can strip these tags out.

I like kguske's level headed response earlier that at least for core PHP-Nuke, maybe not filter (strip?) certain data coming out from the DB for areas where we know only admins have access to. But, of course, it only takes one poorly written add-on module to expose the entire db to the infusing of "bad" code and then its exposed. But, I guess, even my three points above would still not solve that problem.

spottedhog, this is good stuff and should be taken up for serious consideration, IMO, for future patches.

_________________
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 Visit poster's website
montego
PostPosted: Mon Sep 25, 2006 6:53 am Reply with quote

Perfect case in point:
Only registered users can see links on this board! Get registered or login!
 
fkelly
Former Moderator in Good Standing


Joined: Aug 30, 2005
Posts: 3312
Location: near Albany NY

PostPosted: Mon Sep 25, 2006 8:18 am Reply with quote

I have a couple questions about some of your earlier points Spotted. I am just trying to learn this stuff so forgive the "basics" but:

Quote:
Maybe I am crazy, but I saw a big advantage of using this system versus using addslashes, stripslashes, check_html, and htmlentities. One thing I saw was that the InputFilter system actually removes not allowed html from the data input, whereas something like htmlentities just renames certain characters.


Like you, I had some problems posting PHP code to the Forums recently and I was thinking "if you just had something similar to a PRE tag and you posted your code in that block why couldn't you put anything in?" So when I saw your statement about htmlentities I thought "in some cases "renaming" is not all that bad". And what I meant is that if we globally change all the < and >'s that surround tags within a block that we want to preserve why would that not eliminate ALL risk of someone hacking via that? Yeah, you couldn't do bolding or italics either but who needs that in code. The browser would read the block and it wouldn't see any tags so there'd be no risk of something being executed ... or am I missing something?

Next you say:

Quote:
IMHO there are 3 types of database inputs in Nuke.

1. Integers
2. Text with html tags
3. Text without html tags


And my question is whether in #2 you mean text with "allowable html" tags or none at all. And I guess this gets back to my block question before, whether we should have a category, say for admins, of text with non-filtered html tags and another that's filtered for allowable ones. So the categories would be:

1. Integers
2. Text with unfiltered html tags
3. Text with only allowable html tags
4. Text without html tags

Maybe that's what's implicitly meant, I'm not sure.

Just as a final thought, the question of how we deal with Forum filtering is also looming over. If I'm not mistaken that's a different beast from the rest of Nuke and I don't know if anyone on the Nuke side wants to get into ripping apart their code especially in light of the need to periodically do bbtonuke updates when they do a new release. But that's a tangential consideration.
 
View user's profile Send private message Visit poster's website
spottedhog
PostPosted: Mon Sep 25, 2006 9:03 am Reply with quote

Smile Montego.... Interesting point about what to do about certain things.... There is an easy fix for that.... Create a standard block instead of creating from within the blocks admin section. I guess I prefer to error on the side of being more protective than in allow potential issues. You could always allow more html tags if you wanted, however, something like you were mentioning may be better address using static blocks or modules, thus bypassing the filters.

fkelly... To reply to your initial question, my personal opinion is that you may be considering something that is going to be a rare exception to the rule. If for some reason you may need to show data in a way that needs to bypass the filtering, then maybe a new module or index.php file could be created and renamed, and then the sql query could come from that leaving out the filter.

For the 2nd part, if you used htmlentities to rename everything, then something simple like the paragraph tag would also be renamed. This would lead to some big issues in many things. I guess this very point is what sent me to schooling on all this stuff because when the latest patches came out, wow.... html stripped and not allow line breaks or paragraphs. Of course all the words were lumped into one block and italics and bold for highlighting were gone. Took me a long time to figure it out, which led to this whole issue.

On the Forums side...... Smile you all can fight over what to do with phpBB.... Smile I am using SMF.... no such issues or security concerns there.... Smile
 
evaders99
PostPosted: Mon Sep 25, 2006 9:09 am Reply with quote

There are security concerns with SMF too.. just isn't as a popular target. I know, I just upgraded to SMF 1.1RC3
 
fkelly
PostPosted: Mon Sep 25, 2006 9:20 am Reply with quote

Thanks Spotted, you shortened my learning curve on htmlentities quite a bit. I can see what you mean. I just didn't think of paragraphs and line breaks initially.

On the issue of categories of filtering, I'm not sure we are quite meshing yet. How would you classify the wysiwyg editor that comes with RN? Certain html is allowed, certain is filtered. Would that be "text with html tags" in your classification, with the filtering implicit?
 
Gremmie
Former Moderator in Good Standing


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

PostPosted: Mon Sep 25, 2006 10:52 am Reply with quote

Why create objects of these class types in every function? I haven't studied your code closely, but it doesn't appear to me that they have any state besides the bad tags arrays. Can't you create objects once at the "beginning", kind of like the $db object?

I'm all for object oriented programming, but it seems like you could have just as well made these free functions that use global data (the bad tags arrays) since they don't seem to maintain state between calls. These things are going to be used a lot. All those news and constructor calls seem unnecessary for what you are doing.

In general though it does seem a more coherent approach than what we have now. I will study it some more.

_________________
Only registered users can see links on this board! Get registered or login! - An Event Calendar for PHP-Nuke
Only registered users can see links on this board! Get registered or login! - A Google Maps Nuke Module 
View user's profile Send private message
spottedhog
PostPosted: Mon Sep 25, 2006 10:56 am Reply with quote

evaders....... Where have you seen or read or whatever on any security issues with SMF? I have not seen anything posted..... I am not sure I like the stripslashes, etc. in SMF, but it appears to be working.

fkelly.... I would think fields like titles or categories, those that are not currently using a text area would be the nohtml filtering. The Nuke wysiwyg Editor... that is something I need to check on the next day or so..... To me it would not matter what tags were allowed in it because the html filtering would remove any tags over what the editor allowed. For example, the blockquote... could be allowed in the editor, but the filter would take it out. The best scenario would be to keep the allowed tags the same, which is what I have tried to do.... In other words, I took the tags in the $AllowableHTML and individually placed them in the tags array of the InputFilter code. Nuke editor uses the exact same tags.

One advantage of using code such as this InputFilter is that it does much more than block unwanted html tags. It filters other xss code. And it "escapes" the data. Currently, the check_html function does not do escaping..... not sure how much xss stuff it may keep out. addslashes and stripslashes would no longer be needed, nor would htmlentities.

Bottom line is, fields without html, like titles or categories, will not have a Nuke Editor for their inputs. The other text fields could have... Keeping the tags the same between the 2 would be what is needed.... but the filter would be the last gate so to speak.


Last edited by spottedhog on Mon Sep 25, 2006 11:08 am; edited 2 times in total 
spottedhog
PostPosted: Mon Sep 25, 2006 11:00 am Reply with quote

Gremmie... I may have been doing something wrong, but I tried many variations to get "things" to globally recognize the $myFilter. Every --d***-- thing I tried just would not work. If someone could provide a way to globally call it.... of course I would jump on it..... Smile

ohhhhhh..... btw..... it isn't my code...... Smile
 
spottedhog
PostPosted: Mon Sep 25, 2006 11:40 am Reply with quote

Message to all:

I am not promoting or pushing for the entire Nuke community to use that Input Filter. I found it in my search to address the check_html and stripslash issue in the Patch files. For my purposes, it appears to work nicely. And if the PHP Security gurus say to Initial variables, Filter Input, and Escape Output, then this InputFilter does 2 out of the 3. Then it is just a matter of opening variables to an empty array for edit and display functions.

I placed the code and directions on how to use it so if any of you feel it has merit, the code is out there in the open to use as desired. If any of you use it, or the methods I listed, then maybe this is my contribution for all the learning I have gotten from the various communities.

thanks,

spottedhog

Go Boilers! (Notre Dame this week....) Smile
 
Guardian2003
Site Admin


Joined: Aug 28, 2003
Posts: 6793
Location: Ha Noi, Viet Nam

PostPosted: Mon Sep 25, 2006 3:36 pm Reply with quote

Quote:
Where have you seen or read or whatever on any security issues with SMF?
Only registered users can see links on this board! Get registered or login!
 
View user's profile Send private message Send e-mail
spottedhog
PostPosted: Mon Sep 25, 2006 4:04 pm Reply with quote

That vulnerability is to the bridge between Mambo/Joomla and SMF for syncing members/users table. That was not anything to do with the SMF code....
Only registered users can see links on this board! Get registered or login!

It is similar to the phpBB Forum/admin/admin ..... blah blah blah that our sites get hit with daily.

There was one hole to patch about the beginning of the year, but that is all I have seen in the 18 months or so of weekly reading the SMF community forums.

The latest release, 1.1RC3 put in captcha, but requiring email confirmation is better since the captcha is not very readable.

Maybe there are other security issues, but there are not being report or updates being put out.

I do agree that if SMF picks up in popularity, of course the hackers will work on them, but the SMF developers seem to work methodically to get things right.
 
montego
PostPosted: Tue Sep 26, 2006 5:58 am Reply with quote

Quote:
Montego.... Interesting point about what to do about certain things.... There is an easy fix for that.... Create a standard block instead of creating from within the blocks admin section. I guess I prefer to error on the side of being more protective than in allow potential issues. You could always allow more html tags if you wanted, however, something like you were mentioning may be better address using static blocks or modules, thus bypassing the filters.


That is an easy fix for your and I, but we get tons of these types of questions in the forums here. "Why can't I or my admins post _______". However, I DO agree with you that we are probably stuck with having to sway to the conservative side.

And, on that note, I would recommend not stripping out the core nuke check_html, FixQuote, etc. functions, but, rather, either have them "call" the new inputFilter where appropriate and/or just leave them in for compatibility purposes with other add-ons. This gets back to your "Best Practices" statement: somehow making the development community very aware of possibly the "new and better way of doing things".
 
spottedhog
PostPosted: Tue Sep 26, 2006 9:00 am Reply with quote

Smile In my 50 yrs. on God's green earth I have learned that one cannot be everything for everyone, whether it is personalities, business, and especially software. lol. I know support sites are filled with those who wish to do things not built in, but that is a matter of individuals wanting more than can best be provided.

If Nuke code could be near perfect, yet only cover 90% of all user needs, then I would consider that a success.

One can never satisfy all people everywhere. Those wishing to go outside the capabilities need to understand their request is not a developers' emergency, and I think we all understand.

I think CPG Nuke has a good concept, and it is one I am basically following..... and that is make code just a bit different where you have control over not only the core module/block/theme code, but also over 3rd party addons. This is where Nuke tends to get in a bit of trouble, having "too many chefs in the kitchen". 3rd party developers do not follow Best Practices sorry to say.... But that is a function of "The Creator" and not so much the serfs in the nuke kingdon. Smile
 
technocrat
Life Cycles Becoming CPU Cycles


Joined: Jul 07, 2005
Posts: 511

PostPosted: Thu Sep 28, 2006 3:27 pm Reply with quote

We already did was spottedhog is trying to do in v1 of Evo. All you need to do is nest the class inside of if (!defined('ADMIN_FILE')) { Wink

If you wanted to get even more open you could also use is_admin().

For me and the Evo project we are going to stop trying to fix everything in Nuke. It's just too much work. So we are going to complete ditch Nuke and start over from scratch but try to keep backwards compatibility. I feel that its time that our group stops trying to fix things that are too broken and stop supporting someone that can't lead a project out of a paper bag. So v2 will be the last Nuke based release for us. Anything >= v3 will be a totally new CMS.

With that in mind you might want to take a look at this class that I found and have tweaked a bit.

Code:
//Orginal Author: d11wtq (Chris Corbyn)


/*
    Usage:
        $vars = array(
                        array('var', 'type', 'regex', 'where'),
                        array('foo', 'string', '/^foo\d{2}$/i', 'get')
                     );
        $var_handler->expectData($vars);
        $var_handler->getVar('foo');
*/

class variables
{

    var $Vars         = array();
    var $rawVars        = array();
    var $expectVars     = array(); //Variable names
    var $expectLocs     = array(); //POST, GET, COOKIE, REQUEST, SESSION
    var $expectTypes    = array(); //Data types
    var $expectMatches  = array(); //PCRE
   
    function variables() { }

    function expectData($array) {
        foreach ($array as $k => $a) {
            if (is_array($a) && sizeof($a) >= 2) {
                $params = array_values($a);
                $this->addVar($params, $k);
                $this->addLoc($params, $k);
                $this->addMatch($params, $k);
                $this->addType($params, $k);
            }
        }
        $this->validate();
    }

    function getVar($v) {
        if (isset($this->Vars[$v])) return $this->Vars[$v];
    }

    function getRaw($v) {
        if (isset($this->rawVars[$v])) return $this->rawVars[$v];
    }

    function validate() {
        foreach ($this->expectVars as $k => $v) {
            //It seems this is needed for variable variable in the superglobal scope
            global ${$this->expectLocs[$k]};
           
            if (isset(${$this->expectLocs[$k]}[$v])) {
                $tmp = ${$this->expectLocs[$k]}[$v]; //Read it but don't use it yet
                $this->rawVars[$v] = $tmp;
               
                if ($this->expectTypes[$k] && !$this->checkType($tmp, $this->expectTypes[$k])) $tmp = null;
                if ($this->expectMatches[$k] && !$this->checkMatch($tmp, $this->expectMatches[$k])) $tmp = null;
               
                $this->Vars[$v] = $tmp;
            } else {
                $this->Vars[$v] = null;
                $this->rawVars[$v] = null;
            }
        }
    }

    function checkType($v, $type) {
        switch (strtolower($type)) {
            case 'str':
            case 'string': return is_string($v);
            case 'int':
            case 'integer': return is_int($v);
            case 'float': return is_float($v);
            case 'double': return is_double($v);
            case 'array': return is_array($v);
            case 'object': return is_object($v);
            case 'numeric': return is_numeric($v);
        }
        return false;
    }

    function checkMatch($v, $pattern) {
        if (preg_match($pattern, $v)) {
            return true;
        } else {
            return false;
        }
    }

    function addVar($a, $k) {
        if (isset($a[0])) {
            $this->expectVars[$k] = $a[0];
        } else {
            $this->expectVars[$k] = null;
        }
    }

    function addType($a, $k) {
        if (isset($a[1])) {
            $this->expectTypes[$k] = $a[1];
        } else {
            $this->expectTypes[$k] = null;
        }
    }

    function addMatch($a, $k) {
        if (!empty($a[2])) {
            $this->expectMatches[$k] = $a[2];
        } else {
            $this->expectMatches[$k] = null;
        }
    }

    function addLoc($a, $k) {
        if (!empty($a[3])) {
            $this->expectLocs[$k] = $this->getLocation($a[3]);
        } else {
            $this->expectLocs[$k] = $this->getLocation(0);
        }
    }

    function getLocation($loc)
    {
        switch (strtolower($loc))
        {
            case 'get':
            case '$_get':
            case '_get': return '_GET';
            //
            case 'post':
            case '$_post':
            case '_post': return '_POST';
            //
            case 'cookie':
            case '$_cookie':
            case '_cookie': return '_COOKIE';
            //
            case 'session':
            case '$_session':
            case '_session': return '_SESSION';
            //
            case 'request':
            case '$_request':
            case '_request':
            default: return '_REQUEST';
        }
    }
}

global $var_handler;
$var_handler =& new variables();

_________________
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
gregexp
The Mouse Is Extension Of Arm


Joined: Feb 21, 2006
Posts: 1497
Location: In front of a screen....HELP! lol

PostPosted: Thu Sep 28, 2006 4:15 pm Reply with quote

Hey guys thought Id jump in here, The idea of filtering inputs is simply not a bad idea at all.

Simply put if all inputs into the sql database need to be addressed, Id simply recomend NOT putting it in mainfile and put the include statement in the db/db.php file.

Basically filter it all, You could also add a call to the function is_admin or admin_file, like technocrat has previously stated. Putting the include there would basically leave out the option for someone to install some untested module or something that doesnt have all those filter checks in it. Youll see that it is quite easy to manipulate that file to do exactly what you want. As for fixing nuke problems, you and the rest of us Tech. I think nuke should be fixed, and turned into a new cms, like Ravennuke or something, break away from FB'z concepts and lack of work ethic, ohh yeah anyone wanna pay me to develope a cms that will be hacked in a week?
Just curious Laughing

ok back to topic, Just remember that all filtering should be addressed with clause and substance. Basically dont lose the functionality by securing it so hard noone can do anything, admins and mods should be treated with special case. and as for the whole idea of securing the output from the database, I personally would regret that decision, It lacks the fundamental control you have and could break things that it simply should not. Some of the required inputs may be hard to grab but by blocking all forms of possible script coding, you block a lot more then you know.

Any help you might seek Ill be happy to deal with, Just drop a line.

_________________
For those who stand shall NEVER fall and those who fall shall RISE once more!! 
View user's profile Send private message Send e-mail Visit poster's website AIM Address Yahoo Messenger MSN Messenger ICQ Number
Raven
Site Admin/Owner


Joined: Aug 27, 2002
Posts: 17086

PostPosted: Thu Sep 28, 2006 9:30 pm Reply with quote

kguske wrote:
Just to remind everyone of similar discussion, enjoy Only registered users can see links on this board! Get registered or login!.

You might also enjoy this Only registered users can see links on this board! Get registered or login!. It's almost a year old, but I doubt these tools have changed that significantly. At the time, I liked cyberai best. But, I had already done so much with kses that it didn't make sense to reinvent that wheel.

As to where to use it...that's an interesting discussion for sure.

Only registered users can see links on this board! Get registered or login! killing me

I never read that before. That is great!
 
View user's profile Send private message
technocrat
PostPosted: Thu Sep 28, 2006 10:02 pm Reply with quote

Almost exactly a year ago...and yet here we are.

My personal feeling is that there is no reason to fix Nuke or Fork it.
1) FB is an idiot *points at v8*
2) Nuke's modules are so simplistic that is would be easy to create another (hopefully better) CMS with no FB code and compatibility with previous Nuke modules.
3) It would be better to have a more centeralized effort at creating a foundation for the future of a project. With oh I dont know a completely open development process. One that will take input from users and not hide from them.

Alas I am getting on my soap box. If anyone wants to pitch ideas towards the dev process I have opened a forum for it. Only registered users can see links on this board! Get registered or login!
 
Raven
PostPosted: Thu Sep 28, 2006 10:20 pm Reply with quote

Thanks. We are doing something similar, as are several others. It will be interesting to see what the outcome is Smile
 
fkelly
PostPosted: Fri Sep 29, 2006 8:33 am Reply with quote

Quote:
It would be better to have a more centeralized effort at creating a foundation for the future of a project. With oh I dont know a completely open development process. One that will take input from users and not hide from them.


This is getting a bit away from filtering but I'll say this. I've been privileged to work a bit on Raven's releases which I would describe as an open process combined with strong leadership. No one has a monopoly on this like MS so you can't make your "releases" be 5 years apart and maintain user interest, so you have to proceed incrementally (my opinion) and that means you have to pick a limited set of things to work on for each release. You can have a month of so of open discussion at the start of each cycle but eventually someone has to set a direction for the next release and get the developers working in a coordinated fashion to get there.

I also wouldn't overstate how "easy" all this is even granting the "idiocy" of the original author of Nuke. Just look at the complications and issues in the top of this thread. Earlier this morning I was looking at some screens in the "new" release of RN that line up perfectly in Firefox but not in IE. Why? Most likely inconsistent support for CSS standards in IE as opposed to Firefox. Just tell the users to use Firefox? Not an option -- so someone's going to have to go digging into CSS incompatibilities and recode sections of the screen. Is that easy? I don't think so and it's the tip of the iceberg in terms of what goes into a release like this.
 
technocrat
PostPosted: Fri Sep 29, 2006 10:58 am Reply with quote

I never said this was going to be easy. In fact far from it for sure. Having worked on Evo now for a year and half and made 2 full releases has shown me how much time and effort it can take. But the end result I think was well worth it as we have done a number of revolutionary things to Nuke. Including making things like UNION attacks nearly impossible and XSS attacks very difficult. AJAX module and block admins. WYSIWYG class to allow it in any text area. A far more advanced CAPTCHA. A high powered cache system. And more.

So I am far from naive to this. I know most people here have probably not see Evo because of RN, but we have been working on and had a release long before you guys. I am not trying to start something, just pointing out that I just did not walk off the street.

I know this was discussed before and it never materialized into anything, but I figure I would give this a shot again; If anyone is will to start a project from scratch I would be will to pitch in on it. I have many ideas and concepts that I am willing to work with others on. It is by far better to work together than against.

So now that I have helped get this post completely off track killing me
 
Raven
PostPosted: Fri Sep 29, 2006 11:19 am Reply with quote

Just one more off track comment.

technocrat wrote:
I know this was discussed before and it never materialized into anything


The discussion you're referring to was led by me and the purpose was attained, imo. My/Our goal was to basically have a 'think tank' of some of the major players and to see what they were doing and where they were going. Code was shared and some approaches were discussed, etc. The debates were good and respect was always at the forefront, even among some that don't really see eye-to-eye.

We will probably be starting a project after the release and the fall-out from RN2.10. But, it's still being discussed. However, some foundations have already been laid Wink
 
technocrat
PostPosted: Fri Sep 29, 2006 11:30 am Reply with quote

I know I was there in those discussions...that seems like so long ago. I am not saying it was a waste of time, but to me nothing in the form of materials came from it. It was all ideas and concepts, which many of them were not acted upon. So to me nothing materialized.

Anyways I am willing to discuss further joining the project if you have any interest in me. If not I will continue with what I have planned.
 
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 ©