md5 and sha1 hashes now automatically lowercase

cleaned up some code and fixed some vulnerabilities
pull/61/head
Christophe Vandeplas 2012-03-25 15:56:29 +02:00
parent 86b760cd54
commit 7b1673d212
3 changed files with 181 additions and 197 deletions

View File

@ -124,12 +124,8 @@ class EventsController extends AppController {
if (!$this->Event->exists()) {
throw new NotFoundException(__('Invalid event'));
}
// Replaced by isAuthorized
// // only edit own events
// $old_event = $this->Event->read(null, $id);
// if (!$this->_isAdmin() && $this->Auth->user('org') != $old_event['Event']['org']) {
// throw new UnauthorizedException('You are only allowed to edit events of your own organisation.');
// }
// only edit own events verified by isAuthorized
if ($this->request->is('post') || $this->request->is('put')) {
// always force the user and org, but do not force it for admins
if (!$this->_isAdmin()) {
@ -174,12 +170,8 @@ class EventsController extends AppController {
if (!$this->Event->exists()) {
throw new NotFoundException(__('Invalid event'));
}
// Replaced by isAuthorized
// // only edit own events
// $this->Event->read();
// if (!$this->_isAdmin() && $this->Auth->user('org') != $this->Event->data['Event']['org']) {
// throw new UnauthorizedException('You are only allowed to edit your own events.');
// }
// only edit own events verified by isAuthorized
if ($this->Event->delete()) {
$this->Session->setFlash(__('Event deleted'));
$this->redirect(array('action' => 'index'));
@ -204,11 +196,7 @@ class EventsController extends AppController {
$this->Event->id = $id;
$this->Event->read();
// Replaced by isAuthorized
// // only allow alert for own events or admins
// if (!$this->_isAdmin() && $this->Auth->user('org') != $this->Event->data['Event']['org']) {
// throw new UnauthorizedException('You are only allowed to finish events of your own organisation.');
// }
// only allow alert for own events verified by isAuthorized
// fetch the event and build the body
if (1 == $this->Event->data['Event']['alerted']) {

View File

@ -62,13 +62,7 @@ class SignaturesController extends AppController {
public function add($event_id = null) {
if ($this->request->is('post')) {
$this->loadModel('Event');
// Replaced by isAuthorized
// // only own signatures
// $this->Event->recursive = 0;
// $event = $this->Event->findById($this->request->data['Signature']['event_id']);
// if (!$this->_isAdmin() && $this->Auth->user('org') != $event['Event']['org']) {
// throw new UnauthorizedException('You can only add signatures for your own organisation.');
// }
// only own signatures verified by isAuthorized
// Give error if someone tried to submit a signature with attachment or malware-sample type.
// FIXME this is bad ... it should rather by a messagebox or should be filtered out on the view level
@ -190,16 +184,11 @@ class SignaturesController extends AppController {
public function add_attachment($event_id = null) {
if ($this->request->is('post')) {
$this->loadModel('Event');
// // Replaced by isAuthorized
// // // only own signatures
// // $this->Event->recursive = 0;
// // $event = $this->Event->findById($this->request->data['Signature']['event_id']);
// // if (!$this->_isAdmin() && $this->Auth->user('org') != $event['Event']['org']) {
// // throw new UnauthorizedException('You can only add signatures for your own organisation.');
// // }
// only own signatures verified by isAuthorized
// Check if there were problems with the file upload
$filename = Sanitize::HTML($this->request->data['Signature']['value']['name']);
// only keep the last part of the filename, this should prevent directory attacks
$filename = basename($this->request->data['Signature']['value']['name']);
$tmpfile = new File($this->request->data['Signature']['value']['tmp_name']);
if ((isset($this->request->data['Signature']['value']['error']) && $this->request->data['Signature']['value']['error'] == 0) ||
(!empty( $this->request->data['Signature']['value']['tmp_name']) && $this->request->data['Signature']['value']['tmp_name'] != 'none')
@ -265,10 +254,10 @@ class SignaturesController extends AppController {
if($this->request->data['Signature']['malware']) {
// TODO check if CakePHP has no easy/safe wrapper to execute commands
$exec_retval = ''; $exec_output = array();
rename($file->path, $file_in_zip->path); // FIXME prevent any attacks
rename($file->path, $file_in_zip->path); // TODO check if no workaround exists for the current filtering mechanisms
exec("zip -j -P infected ".$zipfile->path.' "'.addslashes($file_in_zip->path).'"', $exec_output, $exec_retval);
if($exec_retval != 0) { // not EXIT_SUCCESS
$this->Session->setFlash(__('Problem with zipping the attachment. Please report to administrator. ', true), 'default', array(), 'error');
$this->Session->setFlash(__('Problem with zipping the attachment. Please report to administrator. '.$exec_output, true), 'default', array(), 'error');
// remove the entry from the database
$this->Signature->delete();
$file_in_zip->delete();
@ -305,12 +294,7 @@ class SignaturesController extends AppController {
if (!$this->Signature->exists()) {
throw new NotFoundException(__('Invalid signature'));
}
// Replaced by isAuthorized
// // only own signatures
// $this->Signature->read();
// if (!$this->_isAdmin() && $this->Auth->user('org') != $this->Signature->data['Event']['org']) {
// throw new UnauthorizedException('You can only edit signatures from your own organisation.');
// }
// only own signatures verified by isAuthorized
$this->Signature->read();
$event_id = $this->Signature->data['Signature']['event_id'];
@ -324,14 +308,12 @@ class SignaturesController extends AppController {
$this->set('attachment', false);
}
debug($this->Signature->data);
if ($this->request->is('post') || $this->request->is('put')) {
if ($this->Signature->save($this->request->data)) {
// TODO should I check here if it's possible to change the event_id or org ?
// say what fields are to be updated
$fieldList=array('category', 'type', 'value', 'to_ids');
if ($this->Signature->save($this->request->data, true, $fieldList)) {
$this->Session->setFlash(__('The attribute has been saved'));
debug($this->request->data);
$this->redirect(array('controller' => 'events', 'action' => 'view', $event_id));
} else {
$this->Session->setFlash(__('The attribute could not be saved. Please, try again.'));
@ -365,12 +347,7 @@ class SignaturesController extends AppController {
if (!$this->Signature->exists()) {
throw new NotFoundException(__('Invalid attribute'));
}
// Replaced by isAuthorized
// // only own signatures
// $this->Signature->read();
// if (!$this->_isAdmin() && $this->Auth->user('org') != $this->Signature->data['Event']['org']) {
// throw new UnauthorizedException('You can only delete signatures from your own organisation.');
// }
// only own signatures verified by isAuthorized
// attachment will be deleted with the beforeDelete() function in the Model
if ($this->Signature->delete()) {

View File

@ -180,6 +180,22 @@ class Signature extends AppModel {
}
}
function beforeValidate() {
// remove leading and trailing blanks
$this->data['Signature']['value'] = trim($this->data['Signature']['value']);
switch($this->data['Signature']['type']) {
// lowercase these things
case 'md5':
case 'sha1':
$this->data['Signature']['value'] = strtolower($this->data['Signature']['value']);
break;
}
// always return true, otherwise the object cannot be saved
return true;
}
function validateSignatureValue ($fields) {
$value = $fields['value'];
$event_id = $this->data['Signature']['event_id'];
@ -188,29 +204,31 @@ class Signature extends AppModel {
$category = $this->data['Signature']['category'];
// check if the signature already exists in the same event
$params = array('recursive' => 0,
'conditions' => array('Signature.event_id' => $event_id,
$conditions = array('Signature.event_id' => $event_id,
'Signature.type' => $type,
'Signature.to_ids' => $to_ids,
'Signature.category' => $category,
'Signature.value' => $value),
'Signature.value' => $value
);
if (isset($this->data['Signature']['id']))
$conditions['Signature.id !='] = $this->data['Signature']['id'];
$params = array('recursive' => 0,
'conditions' => $conditions,
);
if (0 != $this->find('count', $params) )
return 'Attribute already exists for this event.';
// check data validation
switch($this->data['Signature']['type']) {
// FIXME lowercase hashes
case 'md5':
if (preg_match("#^[0-9a-f]{32}$#", $value))
return true;
return 'Checksum has invalid lenght or format. Please double check the value or select "other" for a type.';
return 'Checksum has invalid length or format. Please double check the value or select "other" for a type.';
break;
case 'sha1':
if (preg_match("#^[0-9a-f]{40}$#", $value))
return true;
return 'Checksum has invalid lenght or format. Please double check the value or select "other" for a type.';
return 'Checksum has invalid length or format. Please double check the value or select "other" for a type.';
break;
case 'filename':
// no newline
@ -221,6 +239,7 @@ class Signature extends AppModel {
// no newline
if (!preg_match("#^.*|[0-9a-f]{32}$#", $value))
return true;
return 'Checksum has invalid length or format. Please double check the value or select "other" for a type.';
break;
case 'ip-src':
$parts = explode("/", $value);