Major improvements in the security of the CAPTCHA system (no SQL injection or anything like that); fixed denied form submission due to _af_acting on form object wrongly switched to true
authorDan
Sat, 17 Nov 2007 20:31:01 -0500
changeset 263 d57af0b0302e
parent 262 596945fa6e56
child 264 0fe1f610698d
Major improvements in the security of the CAPTCHA system (no SQL injection or anything like that); fixed denied form submission due to _af_acting on form object wrongly switched to true
includes/clientside/static/autocomplete.js
includes/clientside/static/autofill.js
includes/comment.php
includes/sessions.php
plugins/SpecialUserFuncs.php
--- a/includes/clientside/static/autocomplete.js	Sat Nov 17 18:54:13 2007 -0500
+++ b/includes/clientside/static/autocomplete.js	Sat Nov 17 20:31:01 2007 -0500
@@ -1,5 +1,6 @@
 /*
  * Auto-completing page/username fields
+ * NOTE: A more efficient version of the username field is used for Mozilla browsers. The updated code is in autofill.js.
  */
 
 // The ultimate Javascript app: AJAX auto-completion, which responds to up/down arrow keys, the enter key, and the escape key
--- a/includes/clientside/static/autofill.js	Sat Nov 17 18:54:13 2007 -0500
+++ b/includes/clientside/static/autofill.js	Sat Nov 17 20:31:01 2007 -0500
@@ -1,5 +1,6 @@
 /**
- * Javascript auto-completion for form fields.
+ * Javascript auto-completion for form fields. This supercedes the code in autocomplete.js for MOZILLA ONLY. It doesn't seem to work real
+ * well with other browsers yet.
  */
  
 var af_current = false;
@@ -245,8 +246,12 @@
           form._af_acting = false;
           return true;
         }
+        else
+        {
+          form._af_acting = true;
+          return true;
+        }
       }
-      form._af_acting = true;
     }
   }
   
@@ -255,8 +260,9 @@
     var key = this.event.keyCode;
     if ( key == this.KEY_ENTER && !this.repeat )
     {
+      submitAuthorized = true;
       var form = findParentForm($(this.field_id).object);
-        form._af_acting = false;
+      form._af_acting = false;
       return true;
     }
     switch(key)
@@ -420,6 +426,7 @@
         setTimeout('var body = document.getElementsByTagName("body")[0]; var div = document.getElementById("'+div.id+'"); if ( div ) body.removeChild(div);', 20);
       delete(this.boxes[i]);
     }
+    this.boxes = new Array();
     this.box_id = false;
     this.state = false;
   }
@@ -432,6 +439,7 @@
     else if ( this.state )
       ta.value = this.state;
     this.destroy();
+    findParentForm($(this.field_id.object))._af_acting = false;
   }
   
   this.sleep = function()
--- a/includes/comment.php	Sat Nov 17 18:54:13 2007 -0500
+++ b/includes/comment.php	Sat Nov 17 20:31:01 2007 -0500
@@ -270,6 +270,7 @@
           $real_code = $session->get_captcha($data['captcha_id']);
           if ( $real_code != $data['captcha_code'] )
             $errors[] = 'The confirmation code you entered was incorrect.';
+          $session->kill_captcha();
         }
         
         if ( count($errors) > 0 )
--- a/includes/sessions.php	Sat Nov 17 18:54:13 2007 -0500
+++ b/includes/sessions.php	Sat Nov 17 20:31:01 2007 -0500
@@ -2425,15 +2425,30 @@
   
   function make_captcha($len = 7)
   {
-    $chars = array('A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', '1', '2', '3', '4', '5', '6', '7', '8', '9');
-    $s = '';
-    for($i=0;$i<$len;$i++) $s .= $chars[mt_rand(0, count($chars)-1)];
+    $code = $this->generate_captcha_code($len);
     $hash = md5(microtime() . mt_rand());
     $this->sql('INSERT INTO '.table_prefix.'session_keys(session_key,salt,auth_level,source_ip,user_id) VALUES(\''.$hash.'\', \''.$s.'\', -1, \''.ip2hex($_SERVER['REMOTE_ADDR']).'\', -2);');
     return $hash;
   }
   
   /**
+   * Generates the actual confirmation code text.
+   * @param int String length
+   * @return string
+   */
+  
+  function generate_captcha_code($len = 7)
+  {
+    $chars = array('A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', '1', '2', '3', '4', '5', '6', '7', '8', '9');
+    $s = '';
+    for ( $i = 0; $i < $len; $i++ )
+    {
+      $s .= $chars[mt_rand(0, count($chars)-1)];
+    }
+    return $s;
+  }
+  
+  /**
    * For the given code ID, returns the correct CAPTCHA code, or false on failure
    * @param string $hash The unique ID assigned to the code
    * @return string The correct confirmation code
@@ -2443,18 +2458,24 @@
   {
     global $db, $session, $paths, $template, $plugins; // Common objects
     $s = $this->sql('SELECT salt FROM '.table_prefix.'session_keys WHERE session_key=\''.$db->escape($hash).'\' AND source_ip=\''.ip2hex($_SERVER['REMOTE_ADDR']).'\';');
-    if($db->numrows() < 1) return false;
+    if ( $db->numrows() < 1 )
+    {
+      return false;
+    }
     $r = $db->fetchrow();
+    $db->free_result();
+    $this->sql('DELETE FROM ' . table_prefix . 'session_keys WHERE salt=\'' . $db->escape($r['salt']) . '\';');
     return $r['salt'];
   }
   
   /**
-   * Deletes all CAPTCHA codes cached in the DB for this user.
+   * (AS OF 1.0.2: Deprecated. Captcha codes are now killed on first fetch for security.) Deletes all CAPTCHA codes cached in the DB for this user.
    */
   
   function kill_captcha()
   {
-    $this->sql('DELETE FROM '.table_prefix.'session_keys WHERE user_id=-2 AND source_ip=\''.ip2hex($_SERVER['REMOTE_ADDR']).'\';');
+    // $this->sql('DELETE FROM '.table_prefix.'session_keys WHERE user_id=-2 AND source_ip=\''.ip2hex($_SERVER['REMOTE_ADDR']).'\';');
+    return true;
   }
   
   /**
--- a/plugins/SpecialUserFuncs.php	Sat Nov 17 18:54:13 2007 -0500
+++ b/plugins/SpecialUserFuncs.php	Sat Nov 17 20:31:01 2007 -0500
@@ -353,6 +353,7 @@
     $_GET['coppa'] = ( isset($_POST['coppa']) ) ? $_POST['coppa'] : 'x';
     
     $captcharesult = $session->get_captcha($_POST['captchahash']);
+    $session->kill_captcha();
     if($captcharesult != $_POST['captchacode'])
     {
       $s = 'The confirmation code you entered was incorrect.';
@@ -716,7 +717,10 @@
             
             if(!namegood)
             {
-              if(frm.username.value.match(/^([A-z0-9 \.:\!@\#\*]+){2,}$/ig))
+              <?php
+              // sorry for this ugly hack but jedit gets f***ed otherwise
+              echo 'if(frm.username.value.match(/^([A-z0-9 \.:\!@\#\*]+){2,}$/ig))';
+              ?>
               {
                 document.getElementById('s_username').src='<?php echo scriptPath; ?>/images/unknown.gif';
                 document.getElementById('e_username').innerHTML = '';
@@ -983,19 +987,44 @@
 function page_Special_Captcha()
 {
   global $db, $session, $paths, $template, $plugins; // Common objects
-  if($paths->getParam(0) == 'make')
+  if ( $paths->getParam(0) == 'make' )
   {
     $session->kill_captcha();
     echo $session->make_captcha();
     return;
   }
+  
   $hash = $paths->getParam(0);
-  if(!$hash || !preg_match('#^([0-9a-f]*){32,32}$#i', $hash)) $paths->main_page();
-  $code = $session->get_captcha($hash);
-  if(!$code) die('Invalid hash or IP address incorrect.');
-  require(ENANO_ROOT.'/includes/captcha.php');
+  if ( !$hash || !preg_match('#^([0-9a-f]*){32,32}$#i', $hash) )
+  {
+    $paths->main_page();
+  }
+  
+  // Determine code length
+  $ip = ip2hex($_SERVER['REMOTE_ADDR']);
+  if ( !$ip )
+    die('(very desperate) Hacking attempt');
+  $q = $db->sql_query('SELECT CHAR_LENGTH(salt) AS len FROM ' . table_prefix . 'session_keys WHERE session_key = \'' . $db->escape($hash) . '\' AND source_ip = \'' . $db->escape($ip) . '\';');
+  if ( !$q )
+    $db->_die('SpecialUserFuncs selecting CAPTCHA code');
+  if ( $db->numrows() < 1 )
+    die('Invalid hash or hacking attempt by IP');
+  
+  // Generate code
+  $row = $db->fetchrow();
+  $db->free_result();
+  $len = intval($row['len']);
+  if ( $len < 4 )
+    $len = 7;
+  $code = $session->generate_captcha_code($len);
+  
+  // Update database with new code
+  $q = $db->sql_query('UPDATE ' . table_prefix . 'session_keys SET salt = \'' . $code . '\' WHERE session_key = \'' . $db->escape($hash) . '\' AND source_ip = \'' . $db->escape($ip) . '\';');
+  if ( !$q )
+    $db->_die('SpecialUserFuncs generating new CAPTCHA confirmation code');
+  
+  require ( ENANO_ROOT.'/includes/captcha.php' );
   $captcha = new captcha($code);
-  //header('Content-disposition: attachment; filename=autocaptcha.png');
   $captcha->make_image();
   exit;
 }