Major fixes to the ban system - large IP match lists don't slow down the server miserably anymore.
authorDan
Sun, 18 Nov 2007 18:44:55 -0500
changeset 270 5bcdee999015
parent 269 06db76725891
child 271 f088805540ae
child 272 e0ec986c0af3
child 273 96524a56d475
Major fixes to the ban system - large IP match lists don't slow down the server miserably anymore.
includes/common.php
includes/functions.php
includes/pageutils.php
includes/sessions.php
install.php
plugins/SpecialAdmin.php
plugins/SpecialUserFuncs.php
--- a/includes/common.php	Sat Nov 17 23:30:23 2007 -0500
+++ b/includes/common.php	Sun Nov 18 18:44:55 2007 -0500
@@ -250,6 +250,12 @@
     @call_user_func('page_'.$p[1].'_'.$p[0].'_preloader');
   }
   
+  // One quick security check...
+  if ( !is_valid_ip($_SERVER['REMOTE_ADDR']) )
+  {
+    die('SECURITY: spoofed IP address');
+  }
+  
   $session->start();
   $paths->init();
   
--- a/includes/functions.php	Sat Nov 17 23:30:23 2007 -0500
+++ b/includes/functions.php	Sun Nov 18 18:44:55 2007 -0500
@@ -2727,7 +2727,7 @@
     {
       $array[$i] = decode_unicode_url($val);
     }
-    else
+    else if ( is_array($val) )
     {
       $array[$i] = decode_unicode_array($val);
     }
@@ -2991,6 +2991,72 @@
   return $ips;
 }
 
+/**
+ * Parses a valid IP address range into a regular expression.
+ * @param string IP range string
+ * @return string
+ */
+
+function parse_ip_range_regex($range)
+{
+  // Regular expression to test the range string for validity
+  $regex = '/^(([0-9]+(-[0-9]+)?)(\|([0-9]+(-[0-9]+)?))*)\.'
+           . '(([0-9]+(-[0-9]+)?)(\|([0-9]+(-[0-9]+)?))*)\.'
+           . '(([0-9]+(-[0-9]+)?)(\|([0-9]+(-[0-9]+)?))*)\.'
+           . '(([0-9]+(-[0-9]+)?)(\|([0-9]+(-[0-9]+)?))*)$/';
+  if ( !preg_match($regex, $range) )
+  {
+    return false;
+  }
+  $octets = array(0 => array(), 1 => array(), 2 => array(), 3 => array());
+  list($octets[0], $octets[1], $octets[2], $octets[3]) = explode('.', $range);
+  $return = '^';
+  foreach ( $octets as $octet )
+  {
+    // alternatives array
+    $alts = array();
+    if ( strpos($octet, '|') )
+    {
+      $particles = explode('|', $octet);
+    }
+    else
+    {
+      $particles = array($octet);
+    }
+    foreach ( $particles as $atom )
+    {
+      // each $atom will be either
+      if ( strval(intval($atom)) == $atom )
+      {
+        $alts[] = $atom;
+        continue;
+      }
+      else
+      {
+        // it's a range - parse it out
+        $alt2 = int_range($atom);
+        if ( !$alt2 )
+          return false;
+        foreach ( $alt2 as $neutrino )
+          $alts[] = $neutrino;
+      }
+    }
+    $alts = array_unique($alts);
+    $alts = '|' . implode('|', $alts) . '|';
+    // we can further optimize/compress this by weaseling our way into using some character ranges
+    for ( $i = 1; $i <= 25; $i++ )
+    {
+      $alts = str_replace("|{$i}0|{$i}1|{$i}2|{$i}3|{$i}4|{$i}5|{$i}6|{$i}7|{$i}8|{$i}9|", "|{$i}[0-9]|", $alts);
+    }
+    $alts = str_replace("|1|2|3|4|5|6|7|8|9|", "|[1-9]|", $alts);
+    $alts = '(' . substr($alts, 1, -1) . ')';
+    $return .= $alts . '\.';
+  }
+  $return = substr($return, 0, -2);
+  $return .= '$';
+  return $return;
+}
+
 function password_score_len($password)
 {
   if ( !is_string($password) )
--- a/includes/pageutils.php	Sat Nov 17 23:30:23 2007 -0500
+++ b/includes/pageutils.php	Sun Nov 18 18:44:55 2007 -0500
@@ -23,6 +23,7 @@
   function checkusername($name)
   {
     global $db, $session, $paths, $template, $plugins; // Common objects
+    $name = str_replace('_', ' ', $name);
     $q = $db->sql_query('SELECT username FROM ' . table_prefix.'users WHERE username=\'' . $db->escape(rawurldecode($name)) . '\'');
     if ( !$q )
     {
--- a/includes/sessions.php	Sat Nov 17 23:30:23 2007 -0500
+++ b/includes/sessions.php	Sun Nov 18 18:44:55 2007 -0500
@@ -151,7 +151,7 @@
    */
    
   //var $valid_username = '([A-Za-z0-9 \!\@\(\)-]+)';
-  var $valid_username = '([^<>_&\?\'"%\n\r\t\a\/]+)';
+  var $valid_username = '([^<>&\?\'"%\n\r\t\a\/]+)';
    
   /**
    * What we're allowed to do as far as permissions go. This changes based on the value of the "auth" URI param.
@@ -559,7 +559,7 @@
    * @return string 'success' on success, or error string on failure
    */
    
-  function login_with_crypto($username, $aes_data, $aes_key, $challenge, $level = USER_LEVEL_MEMBER)
+  function login_with_crypto($username, $aes_data, $aes_key_id, $challenge, $level = USER_LEVEL_MEMBER)
   {
     global $db, $session, $paths, $template, $plugins; // Common objects
     
@@ -570,9 +570,9 @@
     
     // Fetch our decryption key
     
-    $aes_key = $this->fetch_public_key($aes_key);
+    $aes_key = $this->fetch_public_key($aes_key_id);
     if(!$aes_key)
-      return 'Couldn\'t look up public key "'.$aes_key.'" for decryption';
+      return 'Couldn\'t look up public key "'.htmlspecialchars($aes_key_id).'" for decryption';
     
     // Convert the key to a binary string
     $bin_key = hexdecode($aes_key);
@@ -587,6 +587,7 @@
     $success = false;
     
     // Escaped username
+    $username = str_replace('_', ' ', $username);
     $db_username_lower = $this->prepare_text(strtolower($username));
     $db_username       = $this->prepare_text($username);
     
@@ -702,6 +703,10 @@
     
     $pass_hashed = ( $already_md5ed ) ? $password : md5($password);
     
+    // Replace underscores with spaces in username
+    // (Added in 1.0.2)
+    $username = str_replace('_', ' ', $username);
+    
     // Perhaps we're upgrading Enano?
     if($this->compat)
     {
@@ -837,7 +842,7 @@
   
   /**
    * Registers a session key in the database. This function *ASSUMES* that the username and password have already been validated!
-   * Basically the session key is a base64-encoded cookie (encrypted with the site's private key) that says "u=[username];p=[sha1 of password]"
+   * Basically the session key is a hex-encoded cookie (encrypted with the site's private key) that says "u=[username];p=[sha1 of password];s=[unique key id]"
    * @param int $user_id
    * @param string $username
    * @param string $password
@@ -896,7 +901,7 @@
   }
   
   /**
-   * Identical to register_session in nature, but uses the old login/table structure. DO NOT use this.
+   * Identical to register_session in nature, but uses the old login/table structure. DO NOT use this except in the upgrade script under very controlled circumstances.
    * @see sessionManager::register_session()
    * @access private
    */
@@ -1338,59 +1343,79 @@
   function check_banlist()
   {
     global $db, $session, $paths, $template, $plugins; // Common objects
-    if($this->compat)
-      $q = $this->sql('SELECT ban_id,ban_type,ban_value,is_regex FROM '.table_prefix.'banlist ORDER BY ban_type;');
-    else
-      $q = $this->sql('SELECT ban_id,ban_type,ban_value,is_regex,reason FROM '.table_prefix.'banlist ORDER BY ban_type;');
-    if(!$q) $db->_die('The banlist data could not be selected.');
-    $banned = false;
-    while($row = $db->fetchrow())
+    $col_reason = ( $this->compat ) ? '"No reason entered (session manager is in compatibility mode)" AS reason' : 'reason';
+    $is_banned = false;
+    if ( $this->user_logged_in )
     {
-      if($this->compat)
-        $row['reason'] = 'None available - session manager is in compatibility mode';
-      switch($row['ban_type'])
+      // check by IP, email, and username
+      $sql = "SELECT $col_reason, ban_value, ban_type, is_regex FROM " . table_prefix . "banlist WHERE \n"
+            . "    ( ban_type = " . BAN_IP    . " AND is_regex = 0 ) OR \n"
+            . "    ( ban_type = " . BAN_IP    . " AND is_regex = 1 AND '{$_SERVER['REMOTE_ADDR']}' REGEXP ban_value ) OR \n"
+            . "    ( ban_type = " . BAN_USER  . " AND is_regex = 0 AND ban_value = '{$this->username}' ) OR \n"
+            . "    ( ban_type = " . BAN_USER  . " AND is_regex = 1 AND '{$this->username}' REGEXP ban_value ) OR \n"
+            . "    ( ban_type = " . BAN_EMAIL . " AND is_regex = 0 AND ban_value = '{$this->email}' ) OR \n"
+            . "    ( ban_type = " . BAN_EMAIL . " AND is_regex = 1 AND '{$this->email}' REGEXP ban_value ) \n"
+            . "  ORDER BY ban_type ASC;";
+      $q = $this->sql($sql);
+      if ( $db->numrows() > 0 )
       {
-      case BAN_IP:
-        if(intval($row['is_regex'])==1) {
-          if(preg_match('#'.$row['ban_value'].'#i', $_SERVER['REMOTE_ADDR']))
+        while ( list($reason, $ban_value, $ban_type, $is_regex) = $db->fetchrow_num() )
+        {
+          if ( $ban_type == BAN_IP && $row['is_regex'] != 1 )
           {
+            // check range
+            $regexp = parse_ip_range_regex($ban_value);
+            if ( !$regexp )
+            {
+              continue;
+            }
+            if ( preg_match("/$regexp/", $_SERVER['REMOTE_ADDR']) )
+            {
+              $banned = true;
+            }
+          }
+          else
+          {
+            // User is banned
             $banned = true;
-            $reason = $row['reason'];
           }
         }
-        else {
-          if($row['ban_value']==$_SERVER['REMOTE_ADDR']) { $banned = true; $reason = $row['reason']; }
-        }
-        break;
-      case BAN_USER:
-        if(intval($row['is_regex'])==1) {
-          if(preg_match('#'.$row['ban_value'].'#i', $this->username))
+      }
+      $db->free_result();
+    }
+    else
+    {
+      // check by IP only
+      $sql = "SELECT $col_reason, ban_value, ban_type, is_regex FROM " . table_prefix . "banlist WHERE
+                ( ban_type = " . BAN_IP    . " AND is_regex = 0 ) OR
+                ( ban_type = " . BAN_IP    . " AND is_regex = 1 AND '{$_SERVER['REMOTE_ADDR']}' REGEXP ban_value )
+              ORDER BY ban_type ASC;";
+      $q = $this->sql($sql);
+      if ( $db->numrows() > 0 )
+      {
+        while ( list($reason, $ban_value, $ban_type, $is_regex) = $db->fetchrow_num() )
+        {
+          if ( $ban_type == BAN_IP && $row['is_regex'] != 1 )
           {
+            // check range
+            $regexp = parse_ip_range_regex($ban_value);
+            if ( !$regexp )
+              continue;
+            if ( preg_match("/$regexp/", $_SERVER['REMOTE_ADDR']) )
+            {
+              $banned = true;
+            }
+          }
+          else
+          {
+            // User is banned
             $banned = true;
-            $reason = $row['reason'];
           }
         }
-        else {
-          if($row['ban_value']==$this->username) { $banned = true; $reason = $row['reason']; }
-        }
-        break;
-      case BAN_EMAIL:
-        if(intval($row['is_regex'])==1) {
-          if(preg_match('#'.$row['ban_value'].'#i', $this->email))
-          {
-            $banned = true;
-            $reason = $row['reason'];
-          }
-        }
-        else {
-          if($row['ban_value']==$this->email) { $banned = true; $reason = $row['reason']; }
-        }
-        break;
-      default:
-        die('Ban error: rule "'.$row['ban_value'].'" has an invalid type ('.$row['ban_type'].')');
       }
+      $db->free_result();
     }
-    if($banned && $paths->get_pageid_from_url() != $paths->nslist['Special'].'CSS')
+    if ( $banned && $paths->get_pageid_from_url() != $paths->nslist['Special'].'CSS' )
     {
       // This guy is banned - kill the session, kill the database connection, bail out, and be pretty about it
       die_semicritical('Ban notice', '<div class="error-box">You have been banned from this website. Please contact the site administrator for more information.<br /><br />Reason:<br />'.$reason.'</div>');
@@ -1402,11 +1427,11 @@
   
   /**
    * Registers a user. This does not perform any type of login.
-   * @param string $username
-   * @param string $password This should be unencrypted.
-   * @param string $email
-   * @param string $real_name Optional, defaults to ''.
-   * @param bool   $coppa     Optional. If true, the account is not activated initially and an admin activation request is sent. The caller is responsible for sending the address info and notice.
+   * @param string New user's username
+   * @param string This should be unencrypted.
+   * @param string E-mail address.
+   * @param string Optional, defaults to ''.
+   * @param bool Optional. If true, the account is not activated initially and an admin activation request is sent. The caller is responsible for sending the address info and notice.
    */
    
   function create_user($username, $password, $email, $real_name = '', $coppa = false)
@@ -1417,6 +1442,7 @@
     $aes = new AESCrypt(AES_BITS, AES_BLOCKSIZE);
     
     if(!preg_match('#^'.$this->valid_username.'$#', $username)) return 'The username you chose contains invalid characters.';
+    $username = str_replace('_', ' ', $username);
     $user_orig = $username;
     $username = $this->prepare_text($username);
     $email = $this->prepare_text($email);
--- a/install.php	Sat Nov 17 23:30:23 2007 -0500
+++ b/install.php	Sun Nov 18 18:44:55 2007 -0500
@@ -119,20 +119,21 @@
 function start_install_table()
 {
   echo '<table border="0" cellspacing="0" cellpadding="0">' . "\n";
+  ob_start();
 }
 
 function close_install_table()
 {
   echo '</table>' . "\n\n";
+  ob_end_flush();
 }
 
 function echo_stage_success($stage_id, $stage_name)
 {
   global $neutral_color;
   $neutral_color = ( $neutral_color == 'A' ) ? 'C' : 'A';
-  ob_start();
   echo '<tr><td style="width: 500px; background-color: #' . "{$neutral_color}{$neutral_color}FF{$neutral_color}{$neutral_color}" . '; padding: 0 5px;">' . htmlspecialchars($stage_name) . '</td><td style="padding: 0 5px;"><img alt="Done" src="images/good.gif" /></td></tr>' . "\n";
-  ob_end_flush();
+  ob_flush();
 }
 
 function echo_stage_failure($stage_id, $stage_name, $failure_explanation, $resume_stack)
@@ -140,9 +141,8 @@
   global $neutral_color;
   
   $neutral_color = ( $neutral_color == 'A' ) ? 'C' : 'A';
-  ob_start();
   echo '<tr><td style="width: 500px; background-color: #' . "FF{$neutral_color}{$neutral_color}{$neutral_color}{$neutral_color}" . '; padding: 0 5px;">' . htmlspecialchars($stage_name) . '</td><td style="padding: 0 5px;"><img alt="Failed" src="images/bad.gif" /></td></tr>' . "\n";
-  ob_end_flush();
+  ob_flush();
   close_install_table();
   $post_data = '';
   $mysql_error = mysql_error();
@@ -378,11 +378,15 @@
   
   $cacheonoff = is_writable(ENANO_ROOT.'/cache/') ? '1' : '0';
   
+  $admin_user = $_POST['admin_user'];
+  $admin_user = str_replace('_', ' ', $admin_user);
+  $admin_user = mysql_real_escape_string($admin_user);
+  
   $schema = file_get_contents('schema.sql');
   $schema = str_replace('{{SITE_NAME}}',    mysql_real_escape_string($_POST['sitename']   ), $schema);
   $schema = str_replace('{{SITE_DESC}}',    mysql_real_escape_string($_POST['sitedesc']   ), $schema);
   $schema = str_replace('{{COPYRIGHT}}',    mysql_real_escape_string($_POST['copyright']  ), $schema);
-  $schema = str_replace('{{ADMIN_USER}}',   mysql_real_escape_string($_POST['admin_user'] ), $schema);
+  $schema = str_replace('{{ADMIN_USER}}',   $admin_user                                    , $schema);
   $schema = str_replace('{{ADMIN_PASS}}',   mysql_real_escape_string($admin_pass          ), $schema);
   $schema = str_replace('{{ADMIN_EMAIL}}',  mysql_real_escape_string($_POST['admin_email']), $schema);
   $schema = str_replace('{{ENABLE_CACHE}}', mysql_real_escape_string($cacheonoff          ), $schema);
@@ -452,6 +456,7 @@
     $key = $aes->hextostring($key);
     $admin_pass = $aes->encrypt($admin_pass, $key, ENC_HEX);
     $admin_user = mysql_real_escape_string($_POST['admin_user']);
+    $admin_user = str_replace('_', ' ', $admin_user);
     
     $q = @mysql_query("UPDATE {$_POST['table_prefix']}users SET password='$admin_pass' WHERE username='$admin_user';");
     if ( !$q )
@@ -1547,7 +1552,7 @@
       start_install_table();
       
       // Are we just trying to auto-rename the config files? If so, skip everything else
-      if ( $_GET['stage'] != 'renameconfig' )
+      if ( !isset($_GET['stage']) || ( isset($_GET['stage']) && $_GET['stage'] != 'renameconfig' ) )
       {
       
         // The stages connect, decrypt, genkey, and parse are preprocessing and don't do any actual data modification.
@@ -1593,18 +1598,29 @@
         $paths->init();
         
         run_installer_stage('initlogs', 'Initialize logs', 'stg_init_logs', '<b>The session manager denied the request to flush logs for the main page.</b><br />
-                             While under most circumstances you can still <a href="install.php?mode=finish">finish the installation</a>, you should be aware that some servers cannot
+                             While under most circumstances you can still <a href="install.php?mode=finish">finish the installation</a> after renaming your configuration files, you should be aware that some servers cannot
                              properly set cookies due to limitations with PHP. These limitations are exposed primarily when this issue is encountered during installation. If you choose
                              to finish the installation, please be aware that you may be unable to log into your site.');
         
+        /*
+         * HACKERS:
+         * If you're making a custom distribution of Enano, put all your custom plugin-related code here.
+         * You have access to the full Enano API as well as being logged in with complete admin rights.
+         * Don't do anything horrendously fancy here, unless you add a new stage (or more than one) and
+         * have the progress printed out properly.
+         */
+        
       } // check for stage == renameconfig
       else
       {
-        // If we did skip that step, set $template_bak to $template to imitate the loading of the Enano API
+        // If we did skip the main installer routine, set $template_bak to make the reversal later work properly
         $template_bak = $template;
       }
 
-      // Final step is to rename the config file      
+      // Final step is to rename the config file
+      // In early revisions of 1.0.2, this step was performed prior to the initialization of the Enano API. It was decided to move
+      // this stage to the end because it will fail more often than any other stage, thus making alternate routes imperative. If this
+      // stage fails, then no big deal, we'll just have the user rename the files manually and then let them see the pretty success message.
       run_installer_stage('renameconfig', 'Rename configuration files', 'stg_rename_config', 'Enano couldn\'t rename the configuration files to their correct production names. Please CHMOD the folder where your Enano files are to 777 and click the retry button below, <b><u>or</u></b> perform the following rename operations and then <a href="install.php?mode=finish">finish the installation</a>.<ul><li>Rename config.new.php to config.php</li><li>Rename .htaccess.new to .htaccess (only if you selected Tiny URLs)</li></ul>');
       
       close_install_table();
--- a/plugins/SpecialAdmin.php	Sat Nov 17 23:30:23 2007 -0500
+++ b/plugins/SpecialAdmin.php	Sun Nov 18 18:44:55 2007 -0500
@@ -2142,17 +2142,14 @@
         }
         if ( $type == BAN_IP )
         {
-          // parse a range of addresses
-          $range = parse_ip_range($entry);
-          if ( !$range )
+          if ( !isset($_POST['regex']) )
           {
-            $error = true;
-            echo '<div class="error-box">Malformed IP address expression.</div>';
-            break;
+            // as of 1.0.2 parsing is done at runtime
+            $entries[] = $entry;
           }
-          foreach ($range as $ip)
+          else
           {
-            $entries[] = $ip;
+            $entries[] = $entry;
           }
         }
         else
@@ -2204,7 +2201,7 @@
   ?>
   Type: <select name="type"><option value="<?php echo BAN_IP; ?>">IP address</option><option value="<?php echo BAN_USER; ?>">Username</option><option value="<?php echo BAN_EMAIL; ?>">E-mail address</option></select><br />
   Rule: <input type="text" name="value" size="30" /><br />
-  <small>You can ban multiple IP addresses, users, or e-mail addresses by separating entries with a single comma (User1,User2). Do not put a space after the comma. For IP addresses, you may specify ranges like 172|192.168.4-30|90-167.1-90, which will turn into 172 and 192 . 168 . 4-30 and 90-167 . 1 - 90, which matches 18,899 IP addresses. Don't specify large ranges (like the example one here) at once or you risk temporarily (~60sec) overloading the server.</small><br />
+  <small>You can ban multiple IP addresses, users, or e-mail addresses by separating entries with a single comma (User1,User2). Do not put a space after the comma. For IP addresses, you may specify ranges like 172|192.168.4-30|90-167.1-90, which will turn into 172 and 192 . 168 . 4-30 and 90-167 . 1 - 90, which matches 18,899 IP addresses.</small><br />
   Reason to show to the banned user: <textarea name="reason" rows="7" cols="40"></textarea><br />
   <input type="checkbox" name="regex" id="regex" />  <label for="regex">This rule is a regular expression</label> (advanced users only)<br />
   <input type="submit" style="font-weight: bold;" name="create" value="Create new ban rule" />
--- a/plugins/SpecialUserFuncs.php	Sat Nov 17 23:30:23 2007 -0500
+++ b/plugins/SpecialUserFuncs.php	Sun Nov 18 18:44:55 2007 -0500
@@ -655,7 +655,7 @@
             if(!namegood)
             {
               //if(frm.username.value.match(/^([A-z0-9 \!@\-\(\)]+){2,}$/ig))
-              var regex = new RegExp('^([^<>_&\?]+){2,}$', 'ig');
+              var regex = new RegExp('^([^<>&\?]+){2,}$', 'ig');
               if ( frm.username.value.match(regex) )
               {
                 document.getElementById('s_username').src='<?php echo scriptPath; ?>/images/unknown.gif';
@@ -717,10 +717,8 @@
             
             if(!namegood)
             {
-              <?php
-              // sorry for this ugly hack but jedit gets f***ed otherwise
-              echo 'if(frm.username.value.match(/^([A-z0-9 \.:\!@\#\*]+){2,}$/ig))';
-              ?>
+              var regex = new RegExp('^([^<>&\?]+){2,}$', 'ig');
+              if ( frm.username.value.match(regex) )
               {
                 document.getElementById('s_username').src='<?php echo scriptPath; ?>/images/unknown.gif';
                 document.getElementById('e_username').innerHTML = '';