Development > Coding and development discussion

!news security?

(1/1)

uberjon:
A while back I had an issue with a guild member, adding random crap to the guild news. Rather  than let him annoy everyone, I proceeded to edit the command permissions


--- Code: --- $this -> register_command('all', 'news', 'GUEST', array('add'=>'MEMBER'));
$this -> register_command('all', 'headline', 'GUEST', array('add'=>'ADMIN'));

--- End code ---
(lines 70-71)

to something like:


--- Code: --- $this -> register_command('all', 'news', 'GUEST', array('add'=>'ADMIN'));
$this -> register_command('all', 'headline', 'GUEST', array('add'=>'ADMIN'));

--- End code ---

Simple fix, right? Nope.. to my amazement somehow, he was still able to add news as he pleased, without being an admin on the bot. Long story short, I disabled the module for a while (was busy). And recently took a look..


--- Code: --- function command_handler($name, $msg, $origin)
{
$com = $this->parse_com($msg);
switch($com['com'])
{
case 'news':
return $this->sub_handler($name, $com, 1);
break;
case 'headline':
return $this->sub_handler($name, $com, 2);
break;
case 'raids':
return $this->sub_handler($name, $com, 3);
break;
default:
$this->error->set("News recieved unknown command '{$com['com']}'.");
return $this->error;
break;
}
}

  function sub_handler($name, $com, $type)
{
switch($com['sub'])
{
case '':
case 'read':
if (($type == 1) || ($type == 2))
return $this->get_news($name);
else
return $this->get_raids($name);
break;
case 'add':
return $this->set_news($name, $com['args'], $type);
break;
case 'del':
case 'rem':
return $this->del_news($name, $com['args']);
break;
default:
//No keywords recognized. Assume that person in attempting to add news and forgot the "add" keyword
$news = "{$com['sub']} {$com['args']}";
return $this->set_news($name, $news, $type);
break;
}
}
--- End code ---

Based on that setup there, a guest (who has access to !news, but not !news add) could 'forget' to use the add portion and default to adding news without re-checking the command permissions!

To fix this, I edited a few things.

replace lines 70-71:

--- Code: --- $this -> register_command('all', 'news', 'MEMBER', array('del'=>'ADMIN','rem'=>'ADMIN','add'=>'ADMIN'));
$this -> register_command('all', 'headline', 'MEMBER', array('add'=>'ADMIN'));

--- End code ---

and replace lines 171-173:


--- Code: --- // edited to not default to 'add' without checking permissions bad!! //No keywords recognized. Assume that person in attempting to add news and forgot the "add" keyword
return $this->get_news($name);

--- End code ---

I hope this may be of some help. (and maybe something similar could be added into the next revision?)

Navigation

[0] Message Index