09 Modtask Updates

[Mindless]

Credits to jaits and Retro

This has been posted and know about for a long time but 09 users may have neglected to check so please add this to your code.

Chances are that most people running 09 source code would not have read the forum in depth so you wont be aware of these patches which have been here a for a long time - Anyway since this is 09 section this fix is a must if you havint done so already, i know from my point of view these were the first things i used to attend to when building my sites. I would also recommend that you check the Secure your cookies thread i done and also apply the ip to cookie mod and also retros hashit system so cookie theft is also killed dead which is here ==>>

@File modtask.php find the select query  :

Code (php) Select Expand

// Fetch current user data...
    $res = mysql_query("SELECT * FROM users WHERE id=".sqlesc($userid)) or sqlerr(__FILE__, __LINE__);
    $user = mysql_fetch_assoc($res) or sqlerr(__FILE__, __LINE__);

Under it add :

   

Code (php) Select Expand

//== Check to make sure your not editing someone of the same or higher class
if ($CURUSER["class"] <= $user['class'] && ($CURUSER['id']!= $userid && $CURUSER["class"] < UC_ADMINISTRATOR))
stderr('Error','You cannot edit someone of the same or higher class.. injecting stuff arent we? Action logged');

Find :

Code (php) Select Expand

if ((isset($_POST['class'])) && (($class = $_POST['class']) != $user['class']))
    {
    if (($CURUSER['class'] < UC_SYSOP) && ($user['class'] >= $CURUSER['class'])) stderr("{$lang['modtask_user_error']}", "{$lang['modtask_try_again']}");

Change to :

Code (php) Select Expand

if ((isset($_POST['class'])) && (($class = $_POST['class']) != $user['class']))
{
if ($class >= UC_SYSOP || ($class >= $CURUSER['class']) || ($user['class'] >= $CURUSER['class']))
stderr("{$lang['modtask_user_error']}", "{$lang['modtask_try_again']}");
if (!is_valid_user_class($class) || $CURUSER["class"] <= $_POST['class']) stderr( ("Error"), "Bad class :P");

change:

Code (php) Select Expand

$updateset[] = "modcomment = " . sqlesc($modcomment);

To:

Code (php) Select Expand

// Add ModComment... (if we changed something we update otherwise we dont include this..)
if (($CURUSER['class'] == UC_SYSOP && ($user['modcomment'] != $_POST['modcomment'] || $modcomment!=$_POST['modcomment'])) || ($CURUSER['class']<UC_SYSOP && $modcomment != $user['modcomment']))
$updateset[] = "modcomment = " . sqlesc($modcomment);


Again change SYSOP in both bits of the IF to whatever you set it at the start...

basically, for sysops, the original behaviour is maintained (you use the POSTED values) but as an extra check, that is updated only if modcomments actually changed compared to the db value... for sysops only, the same issue can occur, but at least now mods and admins (or whatever you set the == in there), the issue is non-existant and modcomments will be recorded properly..

if you do the last bit for the modcomments.. there might be a situation where updateset is empty and the query will fail.. to fix that just do:

Code (php) Select Expand

if (sizeof($updateset)>0)
   mysql_query("UPDATE users SET  " . implode(", ", $updateset) . " WHERE id=$userid") or sqlerr(__FILE__, __LINE__);

this way the query wont even run unless there are changes to be done... (offcourse you'd have no reason to click the button with 0 changes, but anyhow :P )



Next is a page_verify system which will pair scripts and will stop any asshats trying to Xss with dodgy fake scripts  - Links i may add that you shouldnt be clicking anyway, anyone with half a brain would smell a rat but anyway here we go :

Save and upload page_verify.php to /include folder :

Code (php) Select Expand

<?php
// session so that repeated access of this page cannot happen without the calling script.
//
// You use the create function with the sending script, and the check function with the
// receiving script...
//
// You need to pass the value of $task from the calling script to the receiving script. While
// this may appear dangerous, it still only allows a one shot at the receiving script, which
// effectively stops flooding.
// page verify by Retro
     
      class page_verify
      {
      function page_verify ()
      {
      if (session_id () == '')
      {
      session_start ();
      }
      }
    
      function create ($task_name = 'Default')
      {
      global $CURUSER;
      $_SESSION['Task_Time'] = time ();
      $_SESSION['Task'] = md5('user_id:' . $CURUSER['id'] . '::taskname-' . $task_name . '::' . $_SESSION['Task_Time']);
      $_SESSION['HTTP_USER_AGENT'] = $_SERVER['HTTP_USER_AGENT'];
      }
      
      function check ($task_name = 'Default')
      {
      global $CURUSER, $TBDEV, $lang;
      $returl = (isset($CURUSER)?htmlspecialchars($_SERVER['HTTP_REFERER']):$TBDEV['baseurl']."/login.php"); 
      $returl = str_replace('&', '&', $returl); 
      if (isset($_SESSION['HTTP_USER_AGENT']) && $_SESSION['HTTP_USER_AGENT'] != $_SERVER['HTTP_USER_AGENT'])
      stderr("Error", "Please resubmit the form. <a href='".$returl."'>Click HERE</a>",false);
      if ($_SESSION['Task'] != md5('user_id:' . $CURUSER['id'] . '::taskname-' . $task_name . '::' . $_SESSION['Task_Time']))
      stderr("Error", "Please resubmit the form. <a href='".$returl."'>Click HERE</a>",false);
      $this->create ();
      }
      }
?>

Now usage of this is pretty straight forward i'll show one example on userdetails.php and modtask.php - then you can do edit.php/takeedit.php - my.php/takeprofiledit.php login.php/takelogin.php as its the same only difference being name of script

In userdetails.php find :

Code (php) Select Expand

require_once("include/html_functions.php");

Under it add

Code (php) Select Expand

require_once("include/page_verify.php");

Find :

Code (php) Select Expand

$lang = array_merge( load_language('global'), load_language('userdetails') );

Under it add :

Code (php) Select Expand

$newpage = new page_verify();
$newpage->create('modtask');

Now to modtask.php find :

Code (php) Select Expand

require_once("include/user_functions.php");

Under it add

Code (php) Select Expand

require_once("include/page_verify.php");

Find :

Code (php) Select Expand

$lang = array_merge( load_language('global'), load_language('userdetails') );

Under it add :

Code (php) Select Expand

$newpage = new page_verify();
$newpage->check('modtask');

So for you to use it on my/takeprofileedit.php the code placement is exact same only difference will be

Code (php) Select Expand

$newpage = new page_verify();
$newpage->create('takeprofileedit');

And

Code (php) Select Expand

$newpage = new page_verify();
$newpage->check('takeprofileedit');

for you to use it on edit/takeedit.php the code placement is exact same only difference will be

Code (php) Select Expand

$newpage = new page_verify();
$newpage->create('takeedit');

And

Code (php) Select Expand

$newpage = new page_verify();
$newpage->check('takeedit');

Only change is script thats being paired and checked :)

It is not compatible with the existing page_verify class, so if you install it on a system that already uses the older one, then rename this one to page_verify_2 or something similar...

What this one does is to create a keyed array in the $_SESSIONS variable, and to supply that key to the sender script. This is then passed to the receiver script as part of the query string or $_POST, whereby it is compared with the keyed value, and the entry removed, or a forced die() depending on whether they key validated.

The code is designed not to allow more than 10 keys, so a new key will wipe the first key stored. This will technically allow 10 tabbed pages in firefox or 10 browsers in IE. No one should really need more than that.

There is a downside to this code though. Unlike the old class, which can hide the taskname from the member, this one must provide the taskname, so that the checking code knows which key to check. This means that while the class will stop flooding of a take*.php script, it won't stop a single shot attack, using a modified sender script and a fresh taskname, discernable from the source...

With reference to the last post issue, where someone uses the BACK button to return to the COMPOSE page, and sends a second post, you can always run a pre-update query that simply pulls the last userid to post in that topic, and if it matches, bork(). This may require one additional query, but it's not like you have thousands of members continually posting simultaneously, unlike announce where you have thousands of members continually announcing, so you may be able to get away with this simple query.

I do believe that the torrent world as a whole has become complacent, and with all the recent hacks going on, has placed site security at the top of the list, where it bloody well should be...




Credits to creator of this code.

For the 08 posted code fix here it is validated and error free for 09 source although i wouldnt be keen on using it because its dependant on time - session will expire so page_verify in my opinion is better.

Anyway this aint a discusion heres the code :

@File userdetails.php find :

Code (php) Select Expand

$HTMLOUT .= begin_frame("{$lang['userdetails_edit_user']}", true);
      $HTMLOUT .= "<form method='post' action='modtask.php'>\n";

Under it add :

Code (php) Select Expand

require_once "include/validator.php";
$HTMLOUT .= validatorForm("ModTask_$user[id]");

@File modtask.php find :

Code (php) Select Expand

// and verify...
if (!is_valid_id($userid)) stderr("{$lang['modtask_error']}", "{$lang['modtask_bad_id']}");

Under it add :

Code (php) Select Expand

require_once "include/validator.php";
if (!validate($_POST['validator'], "ModTask_$userid" )) die ("Invalid" );


Save and upload validator.php to include folder :

Code (php) Select Expand

<?php
function validator($context){
       global $CURUSER;
       $timestamp=time();
       $hash=hash_hmac("sha1", $CURUSER['secret'], $context.$timestamp);
       return substr($hash, 0, 20).dechex($timestamp);
}
function validatorForm($context){
       return "<input type=\"hidden\" name=\"validator\" value=\"".validator($context)."\"/>";
}

function validate($validator, $context, $seconds=0){
       global $CURUSER;
       $timestamp=hexdec(substr($validator, 20));
       if($seconds && time() > $timestamp + $seconds)
               return False;
       $hash=substr(hash_hmac("sha1", $CURUSER['secret'], $context.$timestamp), 0, 20);
       if (substr($validator, 0, 20) != $hash)
               return False;
       return True;
}
?>

By applying the changes on modtask and using either the page_verify or the validator these two exploits are killed stone dead which i have verified personally.