Ravens PHP Scripts: Forums
 

 

View next topic
View previous topic
Post new topic   Reply to topic    Ravens PHP Scripts And Web Hosting Forum Index -> NukeSentinel(tm) v2.6.x
Author Message
meotoo
Hangin' Around



Joined: Aug 04, 2009
Posts: 36

PostPosted: Thu Jan 07, 2010 2:15 am Reply with quote

Hello Folks,

As i do from time to time, I try to optimize code which i use daily, now here i go when some changes i'm doing to NS, but i'll like your advice/review to ensure i'm not doing something wrong.. in this way both of us can benefit from it Smile

I personally found the get_ip() function a pain, specially i consider the code in bold to be VOID...aka, useless - or i'm wrong? check:

Code:


function get_ip() {
  global $nsnst_const;
[b]-----------------------------------
  if(strpos($nsnst_const['client_ip'], ', ') AND isset($nsnst_const['client_ip'])) {
    $client_ips = explode(', ', $nsnst_const['client_ip']);
    if($client_ips[0] != 'unknown' AND $client_ips[0] != 'none' AND !empty($client_ips[0]) AND !is_reserved($client_ips[0])) {
      if(!ereg("^([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})$", $client_ips[0])) { $client_ips[0] = "none"; }
    } else {
      if(!ereg("^([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})$", $client_ips[1])) { $client_ips[1] = "none"; }
    }
  }
  if(strpos($nsnst_const['forward_ip'], ', ') AND isset($nsnst_const['forward_ip'])) {
    $x_forwardeds = explode(', ', $nsnst_const['forward_ip']);
    if($x_forwardeds[0] != 'unknown' AND $x_forwardeds[0] != 'none' AND !empty($x_forwardeds[0]) AND !is_reserved($x_forwardeds[0])) {
      if(!ereg("^([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})$", $x_forwardeds[0])) { $x_forwardeds[0] = "none"; }
    } else {
      if(!ereg("^([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})$", $x_forwardeds[1])) { $x_forwardeds[1] = "none"; }
    }
  }
  if(strpos($nsnst_const['remote_addr'], ', ') AND isset($nsnst_const['remote_addr'])) {
    $remote_addrs = explode(', ', $nsnst_const['remote_addr']);
    if($remote_addrs[0] != 'unknown' AND $remote_addrs[0] != 'none' AND !empty($remote_addrs[0]) AND !is_reserved($remote_addrs[0])) {
      if(!ereg("^([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})$", $remote_addrs[0])) { $remote_addrs[0] = "none"; }
    } else {
      if(!ereg("^([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})$", $remote_addrs[1])) { $remote_addrs[1] = "none"; }
    }
  }
-----------------------------[/b]
  if(isset($nsnst_const['client_ip']) && !stristr($nsnst_const['client_ip'], "none") && !stristr($nsnst_const['client_ip'], "unknown") AND !is_reserved($nsnst_const['client_ip'])) {
    return $nsnst_const['client_ip'];
  } elseif(isset($nsnst_const['forward_ip']) && !stristr($nsnst_const['forward_ip'], "none") && !stristr($nsnst_const['forward_ip'], "unknown") AND !is_reserved($nsnst_const['forward_ip'])) {
    return $nsnst_const['forward_ip'];
  } elseif(isset($nsnst_const['remote_addr']) && !stristr($nsnst_const['remote_addr'], "none") && !stristr($nsnst_const['remote_addr'], "unknown") AND !is_reserved($nsnst_const['remote_addr'])) {
    return $nsnst_const['remote_addr'];
  } else {
    return "none";
  }
}




Now my fixed/improved/optimized version:

Code:


function get_ip() {
   global $nsnst_const;
   if($nsnst_const['client_ip'] != 'none') {
      $ipaddr = $nsnst_const['client_ip'];
   } else if($nsnst_const['forward_ip'] != 'none') {
      $ipaddr = $nsnst_const['forward_ip'];
   } else if($nsnst_const['remote_addr'] != 'none') {
      $ipaddr = $nsnst_const['remote_addr'];
   } else {
      $ipaddr = 'none';
   }
   return(($ipaddr == 'none' OR is_reserved($ipaddr)) ? 'none' : $ipaddr);
}



Doesn't both functions does the same, being mine 700% faster?.. Cool


Another minimal improvement over the is_(excluded|protected|reserved) functions:

Code:


function is_excluded($rangeip){
  global $prefix, $db;
  $longip = sprintf("%u", ip2long($rangeip));
  $excludenum = $db->sql_numrows($db->sql_query("SELECT c2c FROM `".$prefix."_nsnst_excluded_ranges` WHERE `ip_lo`<='$longip' AND `ip_hi`>='$longip'"));
  return !!$excludenum;
}

function is_protected($rangeip){
  global $prefix, $db;
  $longip = sprintf("%u", ip2long($rangeip));
  $protectnum = $db->sql_numrows($db->sql_query("SELECT c2c FROM `".$prefix."_nsnst_protected_ranges` WHERE `ip_lo`<='$longip' AND `ip_hi`>='$longip'"));
  return !!$protectnum;
}

function is_reserved($rangeip) {
  global $db, $prefix;
  $rangelong = sprintf("%u", ip2long($rangeip));
  $rangenum = $db->sql_numrows($db->sql_query("SELECT c2c FROM `".$prefix."_nsnst_ip2country` WHERE (`ip_lo`<='$rangelong' AND `ip_hi`>='$rangelong') AND `c2c`='01'"));
  return !!$rangenum;
}



What do you guys think?


This for now, but i'm going to post here more fixes, specially related to eregi() usage, i think the ~10 eregi() usage in loops can be replaced by a single preg_match() call, but i'll need your advice as well there as i found those patterns matching a bit strange, for example what's the sense on using "<[^>]script*"? it looks more obvious to use "<[^>]*script" but even doing that, what damage could cause submitting the string "< script"!?...here i assume i must be missing something...

Gretz.
 
View user's profile Send private message Visit poster's website
fkelly
Former Moderator in Good Standing



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

PostPosted: Thu Jan 07, 2010 12:36 pm Reply with quote

The author of NS has moved on to other endeavors and the other person, Raven, who knows the most about it is quite busy. So, I'm not sure how much help you will get in tweaking the zillion details of NS. I'm no expert but on the surface I will say:

[list=]I'm pretty sure the isset tests were put in to avoid notice errors for undefined variables or constants. If you eliminate them you better be testing with error reporting turned up fully and under a number of different circumstances. We don't want to be going backwards and reintroducing notice errors that have been laboriously eliminated over several years.

$client_ips is an array that's derived from exploding $nsnst_const['client_ip'] on a comma. The program then checks to make sure that the IP address is in a proper IP address format (3 digits, a period, three more digits, so on ...) Why do you say this code is void? Your code would allow anything in there.

As to efficiency, it really only matters if this function is called A LOT. I'm not sure if this is the case or not. I'm not saying to be inefficient on purpose but we do have to focus our attention on what counts the most.[/list]
 
View user's profile Send private message Visit poster's website
meotoo







PostPosted: Thu Jan 07, 2010 1:32 pm Reply with quote

Hi fkelly, thx for reply Wink

fkelly wrote:
The author of NS has moved on to other endeavors and the other person, Raven, who knows the most about it is quite busy. So, I'm not sure how much help you will get in tweaking the zillion details of NS. I'm no expert but on the surface I will say:


I know NukeSentinel is no longer maintained by NukeScripts, as we can read on his site it's now maintained by "ravennuke team", i'm not asking Raven here to support, but to someone from the team, last time i asked something about NS was Palbin who brilliantly helped, therefore anybody from the team who isn't busy is ok Smile

fkelly wrote:

I'm pretty sure the isset tests were put in to avoid notice errors for undefined variables or constants. If you eliminate them you better be testing with error reporting turned up fully and under a number of different circumstances. We don't want to be going backwards and reintroducing notice errors that have been laboriously eliminated over several years.


There is the trick! - when get_ip() gets called, it's with the assertion than $nsnst_const['client_ip'], $nsnst_const['forward_ip'], and $nsnst_const['remote_addr'] are either real ip addresses or 'none' string, since they are set at the beginning of the file by using the REGEX_IPV4 pattern.

fkelly wrote:

$client_ips is an array that's derived from exploding $nsnst_const['client_ip'] on a comma. The program then checks to make sure that the IP address is in a proper IP address format (3 digits, a period, three more digits, so on ...) Why do you say this code is void? Your code would allow anything in there.


I found this code VOID because $client_ips (same for $x_forwardeds and $remote_addrs) is not used on that function after the checks, neither are they global variables. (and AFAIK explode() does not return a pointer to the original string address..)

Therefore, what's the sense on doing this:

Code:


if(!ereg("^([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})\\.([0-9]{1,3})$", $client_ips[1])) { $client_ips[1] = "none"; }


if $client_ips is later not used?

Furthermore, as i said above, when get_ip() gets called, we are sure $nsnst_const['client_ip'] is valid (or none) which render those ereg() usage even more useless.

fkelly wrote:

As to efficiency, it really only matters if this function is called A LOT. I'm not sure if this is the case or not. I'm not saying to be inefficient on purpose but we do have to focus our attention on what counts the most.


Well, the get_ip() function is used ONCE... but i know you guys will need to get rid of all those ereg() functions usage and i'm giving my two cents here! - as i said on my first post, both of us we'll benefit from those little improvements i want to do, if i do not make any mistake.. but for the get_ip() function i saw it "mostly" clear.. and even if it's used once, your server will notice it if you get more than 100 visits daily Wink
 
fkelly







PostPosted: Fri Jan 08, 2010 8:39 am Reply with quote

Thanks Meetoo. Those are good points. I did not look into how and from which programs the function get_ip() gets called. Since it is a general purpose function it could be called from anywhere in the NukeSentinel code so we have to be absolutely sure that 'client_ip' is set or run the isset test to make sure. I know that in debugging we often worked backwards with error reporting turned all the way up. If we saw a notice error on a variable not set we'd go in and put the isset test in to stop the notice.

I will put a reference to your post into our issue tracking system for the next release of Ravennuke (after the upcoming 2.40.01 patch release, for which code is frozen) and when someone is working on Sentinel they can take a look. I can tell you that everyone on the team is pretty much in up to their eyeballs with getting the patch release out.

Re-reading your post before sending this off, I will say this as a general matter. When you are writing a general purpose function, such as get_ip() you really have no control over how or from where it is ultimately called. So, even though it is a little extra overhead it makes sense to adopt a belt and suspenders approach and (a) make sure variables you use are set and (b) that they have contents that are valid for their intended use. But yes, we eventually do need to go get rid of or translate all the eregis and there is no point in having one if the result is never used. So we will take your suggestion into account, thanks.
 
meotoo







PostPosted: Fri Jan 08, 2010 1:15 pm Reply with quote

I'm glad to help Smile

I did a search over the whole sources and found the get_ip() function is used only once and from within includes/nukesentinel.php only, i was about to completely remove it, but well.. the tiny version should be ok as well.
 
meotoo







PostPosted: Fri Jan 08, 2010 1:38 pm Reply with quote

erm......i think this function is more correct:

Code:


function get_ip() {
   global $nsnst_const;
   if($nsnst_const['client_ip'] != 'none' AND !is_reserved($nsnst_const['client_ip'])) {
      $ipaddr = $nsnst_const['client_ip'];
   } else if($nsnst_const['forward_ip'] != 'none' AND !is_reserved($nsnst_const['forward_ip'])) {
      $ipaddr = $nsnst_const['forward_ip'];
   } else if($nsnst_const['remote_addr'] != 'none' AND !is_reserved($nsnst_const['remote_addr'])) {
      $ipaddr = $nsnst_const['remote_addr'];
   } else {
      $ipaddr = 'none';
   }
   return($ipaddr);
}


this way, if the first found valid IP is reserved, we'll not return 'none' but check instead for the other two IPs, as the original function does.. sorry for the little mistake Embarassed
 
fkelly







PostPosted: Fri Jan 08, 2010 2:31 pm Reply with quote

Your suggestion is now in our issue tracking system for investigation as part of Ravennuke 2.5. Thanks!
 
Raven
Site Admin/Owner



Joined: Aug 27, 2002
Posts: 17088

PostPosted: Fri Jan 08, 2010 3:07 pm Reply with quote

Good exchange! The core code was written by Bob and myself with some additions by a few others. We have never really gone back over it for cleanup/efficiency as we have focused on the basic adage "if it ain't broke then don't fix it" Wink

There is much more that could be done but NS is becoming less and less needed as the hosts now have many more tools available to them at the server level to help mitigate many of the issues that we set out to do. Having said that, we have no intention of discontinuing NS and as soon as we get 'spare' time we hope to clean it up. And as Frank has said, we are tracking the issue for a future release.
 
View user's profile Send private message
meotoo







PostPosted: Fri Jan 08, 2010 3:35 pm Reply with quote

you're welcome Smile


here i go again with more, this time related to the eregi() used on the "SCRIPTING attack" filter:

from this:

Code:


    foreach($_GET as $sec_key => $secvalue) {
      if((eregi("<[^>]script*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*object*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*iframe*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*applet*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*meta*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]style*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*form*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*img*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]*onmouseover*\"?[^>]*>", $secvalue)) ||
        (eregi("<[^>]body*\"?[^>]*>", $secvalue) && !eregi("<[^>]tbody*\"?[^>]*>", $secvalue)) ||
        (eregi("\([^>]*\"?[^)]*\)", $secvalue)) ||
        (eregi("\"", $secvalue)) ||
        (eregi("forum_admin", $sec_key)) ||
        (eregi("inside_mod", $sec_key))) {
           block_ip($blocker_row);
        }
      }



To this:

Code:


    foreach($_GET as $sec_key => $secvalue) {
      if(strpos($secvalue,'"') OR stripos_clone($sec_key,'forum_admin') OR stripos_clone($sec_key,'inside_mod') OR preg_match('#(\<([^>]*(script|object|iframe|applet|meta|style|form|img|onmouse(over|out))|[^t]*body)"?[^>]*\>|\([^>]*"?[^)]*\))#i',$secvalue)) {
           block_ip($blocker_row);
        }
      }



I know this is quite sensitive code and special attention must be taken, i hope i didn't did any mistake, but no promises Wink

Also, as i said on my initial post,i found those patterns a little strange, additionally to the "script" example on the first post, i'll guest related to the "object" html tag:

Why it must be: "<[^>]*object*\"?[^>]*>" ??

That apply to:

< foobar object>
<foobecttttttttt>
< objecttteeeeeeeuuuu>
<objectionallyjkshadkjaskj>

-- and a lot of other strange combinations --

shouldn't we just avoid the "object" (as-is) html tag? with that in mind is how i've created the preg_match() replacement.

please check it carefully yourself and post here if you found any error on my pattern.

Thanks Smile
 
fkelly







PostPosted: Sat Jan 09, 2010 9:38 am Reply with quote

I'll say this as an individual; it's just my private opinion. I've disabled the scripting blocker entirely in NS. It's really crude and in my experience resulted in way too many false positives ... I had users giving up trying to post things after they got banned once or twice by the scripting filter. It is, again in my private opinion, a prime example of fractured filtering. It can be a fail safe for a module that doesn't filter its own POST data after a form is submitted and doesn't use the wyiswyg facilities or run POST data through the check_html function. But if you are doing these things I really don't think you need the script filter in NS.

If you look at the complexity that something like HTML Purifier or even kses or even the fckeditor has to go through to effectively filter content you can see that trying to encompass a complete POST filter in a single eregi in NS is just impossible. It won't catch everything that is "bad" and it will catch somethings that are "good" or at least "okay".

I'm not sure what direction will be set for this filter in subsequent versions of NS. If we intend to keep it and tweak it then I'd suggest a good first step would be to "decode" the current code into a logical series of English language statements that translate exactly what the eregi is doing into a format that a normal person could understand.

Rereading I see that you've focused on the GET portion of the scripting filter. I haven't had that many problems with false positives on GET's but I think that's because we don't use many GETS in Ravennuke. In my own modules I don't use GETS at all and even if I did I would not really process the data sent by a get or if I did I'd filter it precisely so it could only have a limited range of content. And if I did that I wouldn't need NS's get filter. But very likely I don't understand the full range of the threat NS was responding to.

I guess what I'm struggling to say well is that before we focus on tweaking current NS eregi's or replacing them we ought to step back a bit and look at where NS fits in the overall RN context, what threats it is dealing with and what situations it is needed in and then look at coding solutions as needed.
 
hicuxunicorniobestbuildpc
The Mouse Is Extension Of Arm



Joined: Aug 13, 2009
Posts: 1122

PostPosted: Sat Jan 09, 2010 12:13 pm Reply with quote

All modifications are working properly from nukesentinel.php. tested on ravennuke and phpnuke 8.1. it is working fine. Thanks meotoo. I wonder if you did already the rest of the part like


Code:
      // END - Added by Raven 11/19/2007 to exclude Forums and Private_Message Posting blocks

        foreach($_POST as $secvalue) {
          if((@eregi("<[^>]*iframe*\"?[^>]*", $secvalue)) ||
            (@eregi("<[^>]*object*\"?[^>]*", $secvalue)) ||
            (@eregi("<[^>]*applet*\"?[^>]*", $secvalue)) ||
            (@eregi("<[^>]*meta*\"?[^>]*", $secvalue)) ||
            (@eregi("<[^>]*onmouseover*\"?[^>]*", $secvalue)) ||
            (@eregi("<[^>]script*\"?[^>]*", $secvalue)) ||
            (@eregi("<[^>]body*\"?[^>]*>", $secvalue) && !eregi("<[^>]tbody*\"?[^>]*>", $secvalue)) ||
            (@eregi("<[^>]style*\"?[^>]*", $secvalue))) {
            block_ip($blocker_row);
          }
       }
     }
  }
}
 
View user's profile Send private message
Raven







PostPosted: Sat Jan 09, 2010 12:35 pm Reply with quote

I just set mine for admin notification like this:

Image
 
meotoo







PostPosted: Sat Jan 09, 2010 1:54 pm Reply with quote

fkelly wrote:
--snip--
I'm not sure what direction will be set for this filter in subsequent versions of NS. If we intend to keep it and tweak it then I'd suggest a good first step would be to "decode" the current code into a logical series of English language statements that translate exactly what the eregi is doing into a format that a normal person could understand.


Quite true!

Also, i don't understand why that quote char in the pattern, which can appear Zero or one times ("?) and also if for example we want to avoid the "body" html-tag, how the pattern "<[^>]body" would work... even with that in mind i wanted to create the preg_match pattern with "backward compatibility", but probably we can avoid [^>]* at the beginning of the pattern...

fkelly wrote:

Rereading I see that you've focused on the GET portion of the scripting filter. I haven't had that many problems with false positives on GET's but I think that's because we don't use many GETS in Ravennuke. In my own modules I don't use GETS at all and even if I did I would not really process the data sent by a get or if I did I'd filter it precisely so it could only have a limited range of content. And if I did that I wouldn't need NS's get filter. But very likely I don't understand the full range of the threat NS was responding to.


Yeah, i focused on the GET portion because i like to do things step by step, if the first pattern is found to be ok then i can continue with next stuff Smile

The POST-part pattern will be:

Code:


'#\<([^>]*(script|object|iframe|applet|meta|style|onmouse(over|out))|[^t]*body)"?[^>]*\>#i'


I understand the false-positives you're referring to are caused related to the "object" example i've above posted, there could be a lot of combinations.

of course we (or atleast me) don't want html-tags as-is on GET parameters, what you'd allow on POST requests is really up to everybody's website...

If you do not want false-positives, and/or if you want to be a little more permisive, the correct pattern may would be:

Code:


'#\<((script|object|iframe|applet|([^>]*meta|style|onmouse(over|out)))|[^t]*body)"?[^>]*\>#i'


or maybe better (to my understanding, if we just don't want those html tags):

Code:


'#\<((script|object|iframe|applet|([^>]*meta|style|onmouse(over|out)))|[^t]*body)\s*\=[^>]*\>#i'


(note i've replaced the quote which i dunno why is there..)

and as above stated, @all: please post here if you found some problem on my patterns.


Well, i think that's all for now, if i found something else which worth replacing i'll post it here as well.
 
meotoo







PostPosted: Sat Jan 09, 2010 2:14 pm Reply with quote

Raven wrote:
I just set mine for admin notification like this:

Image


and where do you forward users, if i can ask? just curious, does not really matter Wink

what probably matters is how many "scripting related" emails did you receive(d), and the URLs (get/post) contents, that way we can elaborate a more elegant preg_match()'s pattern.

I need to look at my mail archive to check all this, and the possible false-positives i've received (and indeed i use email&block and write to htaccess), the ones i can remember right now are from feedburner a few days ago, which was false because they just contained parenthesis in the url....
 
fkelly







PostPosted: Sat Jan 09, 2010 2:40 pm Reply with quote

Just a suggestion. If you are looking at filtering, which is what this script code does, spend some time reading the HTML Purifier discussion:

http://htmlpurifier.org/

and, if you are code oriented (as it seems you are) look at /includes/kses/kses.php in our distribution.

When you see what these software "systems" have to do to try to filter out malicious HTML (and other attacks) I think you will agree that trying to accomplish the same thing in a couple of eregis in NukeSentinel is the wrong way to go. Or, to put it more precisely, it was an important "bandaid" for Nuke based systems to have given that the original PHPnuke did almost no filtering and was wide open to exploits. But it's something we need to go beyond.

I was thinking more today about what has happened on my bike club site. Basically people see the nice wysiwyg box and they have something composed in Word (either Word itself or Word as their editor for email) and they copy and paste it into the wyiswyg box and Word puts a whole bunch of lame codes in, especially styles, and the Sentinel filters go nuts and ban em. The users don't have any clue what happened, all they know is that they were trying to post something on our web site and bam "YOU ARE BANNED" and that's about the end of their efforts to post.
 
Raven







PostPosted: Sat Jan 09, 2010 5:35 pm Reply with quote

metoo wrote:
and where do you forward users, if i can ask? just curious, does not really matter


You can forward them to anywhere you want Smile
 
warren-the-ape
Worker
Worker



Joined: Nov 19, 2007
Posts: 196
Location: Netherlands

PostPosted: Thu Sep 16, 2010 9:09 am Reply with quote

Been a while since I visited RavenPHPScripts. Nice to see that things are still up and running Cool
Stumbled upon this thread cause I (or rather my users) sometimes experience false positive scripting attacks as well, mainly in PM's.

I narrowed it down to the use of characters like < and >, but saw that ( ) and - could be responsible for it as well.

One user even reported that removing the quoted section from his PM helped solve the problem. This makes more sense after reading meotoo's comment

meotoo wrote:
Also, i don't understand why that quote char in the pattern, which can appear Zero or one times ("?)


Or wasn't that a reference to the [ quote ] tag?


Noticed that 2.6.03 was released as part of RavenNuke, but minor fixes according to Palbin.

Anyway, here are a few script attacks I received recently, might be helpful to determine what's false positive and what's not.

Code:
Script Name: /index.php

Query String: page=|echo "casper";echo "kae";|
Get String: page=|echo \"casper\";echo \"kae\";|
Post String: Not Available


Code:
Script Name: /modules.php

Query String: name=Forums/modules/Forums/admin/admin_db_utilities.php?phpbb_root_path=|echo "casper";echo "kae";|
Get String: name=Forums/modules/Forums/admin/admin_db_utilities.php?phpbb_root_path=|echo \"casper\";echo \"kae\";|
Post String: Not Available


Code:
Script Name: /modules/Forums/admin/admin_db_utilities.php

Query String: phpbb_root_path=|echo "casper";echo "kae";|
Get String: phpbb_root_path=|echo \"casper\";echo \"kae\";|
Post String: Not Available


Code:
Script Name: /modules.php

Query String: name=Forums&file=viewtopic&p=63357  and length(user())>0
Get String: name=Forums&file=viewtopic&p=63357  and length(user())>0
Post String: Not Available


I get this one the most (from non-registered visitors) but seems harmless to me, probably the parentheses causing it? Sometimes it's p=4:
Code:
Script Name: /modules.php

Query String: name=Forums+(200+ok)+ACCEPTED+p=5
Get String: name=Forums (200 ok) ACCEPTED p=5
Post String: Not Available



Isn't it possible to exclude PM's (private message module) from the script filter?

According to the snippet posted by unicornio that should already be the case, or am I wrong?

unicornio wrote:
Code:
// END - Added by Raven 11/19/2007 to exclude Forums and Private_Message Posting blocks
 
View user's profile Send private message
Susann
Moderator



Joined: Dec 19, 2004
Posts: 3191
Location: Germany:Moderator German NukeSentinel Support

PostPosted: Fri Sep 17, 2010 9:12 am Reply with quote

warren-the-ape In my opinion its dangerous to exclude private messages from the script filter. You can never trust your members.
Maybe Raven will tell us something different but I don´t believe so.
Second it´s great that Caspar is autmatically blocked. That`s what I thought after the first attack from Mama. However, these attacks will not work with RavenNuke but its dangerous for other CMS.

I´m getting the same ban messages like you for a site without an activated forum and I have still not found out directly why. It really doesn´t matter if there is p=4: or 5.

Quote:

User ID: Anonymous (1)
Reason: Abuse-Script
--------------------
Referer: on site
User Agent: Opera/7.11 (Windows NT 5.1; U) [en]
HTTP Host: mysite.com
Script Name: /modules.php
Query String: name=Forums+(200+ok)+ACCEPTED
Get String: name=Forums (200 ok) ACCEPTED
Post String: Not Available
Forwarded For: none
Client IP: none
 
View user's profile Send private message
Raven







PostPosted: Fri Sep 17, 2010 8:35 pm Reply with quote

It's the (). Also, it doesn't matter if the module is active or not because it intercepts the get/post strings before the data is passed to the application, which is exactly what its supposed to do (i.e. a filter).
 
warren-the-ape







PostPosted: Sat Sep 18, 2010 5:02 am Reply with quote

Susann wrote:
warren-the-ape In my opinion its dangerous to exclude private messages from the script filter. You can never trust your members.
Maybe Raven will tell us something different but I don´t believe so.


Okay it intercepts before the data is passed, but doesn't phpbb/bb2nuke (+ *nuke) sanitize that input already?

It's also strange that I've never seen this happen with regular forum posts but only occurs in the private messages module. All of them from legit users with legit messages (well not to NS apparently Very Happy).

Quote:
Second it´s great that Caspar is autmatically blocked. That`s what I thought after the first attack from Mama. However, these attacks will not work with RavenNuke but its dangerous for other CMS.


Yes, they were annoying indeed. There were various '* bot search' user agents requesting and trying to exploit a contact.php file which obviously isn't present in *Nuke.

A few rewrite conditions took care of that Wink
 
Susann







PostPosted: Sat Sep 18, 2010 12:21 pm Reply with quote

Raven thanks !

Quote:
A few rewrite conditions took care of that

@warren-the-ape
You are always free to use such other solutions but at your own risk.Let us know your experience with an excluded PM module.
There is a standard filter in Nuke and phpBB but only together with NukeSentinel I feel very well protected.
Wink
 
Palbin
Site Admin



Joined: Mar 30, 2006
Posts: 2583
Location: Pittsburgh, Pennsylvania

PostPosted: Sat Sep 18, 2010 1:03 pm Reply with quote

warren-the-ape, yes the data is probably properly filtered, but I do not think anyone here is going to give a 100 percent guarantee that it is. At least not at this time.

_________________
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan. 
View user's profile Send private message
warren-the-ape







PostPosted: Sat Sep 18, 2010 2:21 pm Reply with quote

@ Susann & Palbin
Yeah, I understand that turning the filter off does make me more vulnerable, that's why I let it on for now.

Those false positives occur sporadically so it isn't a huge deal, but there probably is something wrong (or off) with the way NS handles private messages.

Quote sections [quote ] in combination with characters like ( ) < > etc. seem to somehow trigger NS's script filter.

Perhaps meotoo (if he's still active) or another dev with some spare time (Wink) could look into this.

@ Susann
I prefer to block the obvious baddies before they hit the site. I believe Kguske or Fkelly once posted a few basic rewrite conditions. Over time I added extra conditions and saved the left-overs (so to speak Razz) to NS.
 
Palbin







PostPosted: Sat Sep 18, 2010 3:12 pm Reply with quote

warren-the-ape, I'll try and take a look at it, but it may be a few days as I am getting my new computer setup. Downloading and installing everything to get up and running again is just a pain Very Happy
 
Raven







PostPosted: Sat Sep 18, 2010 4:32 pm Reply with quote

NS is due for a complete overhaul. It was written at a time when there wasn't much else available. If we all would code with security in mind we wouldn't need NS or at least wouldn't need all of it. The ()<> triggers the filter because those are common in script attacks. So we decided to err on the side of caution. I'd rather get an email advising me of false positive that I glance at and then delete than have a legitimate attack sneak through. If I ever do get the time to rewrite NS I have on my to-do list, amongst other things, to look into offering customization so that each person can decide:

* What modules/blocks to include/exclude (default->same as now)
* What characters to include/exclude in filters (default->same as now)

If you look back on the insecure history of phpbb2 and especially phpbb2+nuke, you will see why we felt we couldn't depend on phpbb2 for our purposes/goals at that time..

See [ Only registered users can see links on this board! Get registered or login! ]
See [ Only registered users can see links on this board! Get registered or login! ]
 
Display posts from previous:       
Post new topic   Reply to topic    Ravens PHP Scripts And Web Hosting Forum Index -> NukeSentinel(tm) v2.6.x

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 ©