diff --git a/app/Controller/AttributesController.php b/app/Controller/AttributesController.php index d6f71c125..1654b5c38 100644 --- a/app/Controller/AttributesController.php +++ b/app/Controller/AttributesController.php @@ -74,7 +74,7 @@ class AttributesController extends AppController { // only own attributes verified by isAuthorized // Give error if someone tried to submit a attribute 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 + // TODO change behavior attachment options - this is bad ... it should rather by a messagebox or should be filtered out on the view level if($this->Attribute->typeIsAttachment($this->request->data['Attribute']['type'])) { $this->Session->setFlash(__('Attribute has not been added: attachments are added by "Add attachment" button', true), 'default', array(), 'error'); $this->redirect(array('controller' => 'events', 'action' => 'view', $this->request->data['Attribute']['event_id'])); @@ -218,14 +218,15 @@ class AttributesController extends AppController { $this->Attribute->create(); if($this->request->data['Attribute']['malware']) { $this->request->data['Attribute']['type'] = "malware-sample"; - $this->request->data['Attribute']['value'] = $filename.'|'.$tmpfile->md5(); // TODO gives problems with bigger files + $this->request->data['Attribute']['value'] = $filename.'|'.$tmpfile->md5(); // TODO gives problems with bigger files + $this->request->data['Attribute']['to_ids'] = 1; // LATER let user choose to send this to IDS } else { $this->request->data['Attribute']['type'] = "attachment"; - $this->request->data['Attribute']['value'] = $filename; + $this->request->data['Attribute']['value'] = $filename; + $this->request->data['Attribute']['to_ids'] = 0; } $this->request->data['Attribute']['uuid'] = String::uuid(); - $this->request->data['Attribute']['to_ids'] = 0; // LATER permit user to send this to IDS $this->request->data['Attribute']['batch_import'] = 0; if ($this->Attribute->save($this->request->data)) { @@ -316,7 +317,7 @@ class AttributesController extends AppController { if('attachment' == $this->Attribute->data['Attribute']['type'] || 'malware-sample'== $this->Attribute->data['Attribute']['type'] ) { $this->set('attachment', true); - // FIXME we should ensure value cannot be changed here and not only on a view level (because of the associated file) + // TODO we should ensure 'value' cannot be changed here and not only on a view level (because of the associated file) // $this->Session->setFlash(__('You cannot edit attachment attributes.', true), 'default', array(), 'error'); // $this->redirect(array('controller' => 'events', 'action' => 'view', $old_attribute['Event']['id'])); } else { diff --git a/app/Controller/EventsController.php b/app/Controller/EventsController.php index 9a2848bb9..065d6533c 100644 --- a/app/Controller/EventsController.php +++ b/app/Controller/EventsController.php @@ -85,13 +85,13 @@ class EventsController extends AppController { $relatedAttributes = array(); $this->loadModel('Attribute'); $fields = array('Attribute.id', 'Attribute.event_id', 'Attribute.uuid'); - foreach ($this->Event->data['Attribute'] as $key => $attribute) { + foreach ($this->Event->data['Attribute'] as &$attribute) { $relatedAttributes[$attribute['id']] = $this->Attribute->getRelatedAttributes($attribute, $fields); // for REST requests also add the encoded attachment if ($this->_isRest() && $this->Attribute->typeIsAttachment($attribute['type'])) { // LATER check if this has a serious performance impact on XML conversion and memory usage $encoded_file = $this->Attribute->base64EncodeAttachment($attribute); - $this->Event->data['Attribute'][$key]['data'] = $encoded_file; + $attribute['data'] = $encoded_file; } } $this->set('relatedAttributes', $relatedAttributes); @@ -231,8 +231,8 @@ class EventsController extends AppController { $fieldList=array('date', 'risk', 'info', 'published', 'private'); // always force the org, but do not force it for admins if ($this->_isAdmin()) { - $this->Event->read(); // FIXME URGENT this should be deleted? delete and test - $fieldList[]='org'; + // set the same org as existed before + $this->Event->read(); $this->request->data['Event']['org'] = $this->Event->data['Event']['org']; } // we probably also want to remove the published flag @@ -509,8 +509,6 @@ class EventsController extends AppController { if ($this->request->is('post') || $this->request->is('put')) { $message = $this->request->data['Event']['message']; if ($this->_sendContactEmail($id, $message)) { - // LATER when a user is deleted this will create problems. - // LATER send the email to all the people who are in the org that created the event // redirect to the view event page $this->Session->setFlash(__('Email sent to the reporter.', true)); } else { @@ -528,7 +526,7 @@ class EventsController extends AppController { /** * - * Sends out an email to all people within the same group + * Sends out an email to all people within the same org * with the request to be contacted about a specific event. * @todo move _sendContactEmail($id, $message) to a better place. (components?) * @@ -709,7 +707,7 @@ class EventsController extends AppController { // check if the key is valid -> search for users based on key $this->loadModel('User'); // no input sanitization necessary, it's done by model - // TODO do not fetch recursive + // do not fetch recursive $this->User->recursive=0; $user = $this->User->findByAuthkey($key); if (empty($user)) { @@ -742,9 +740,9 @@ class EventsController extends AppController { $sid++; switch ($attribute['type']) { - // LATER test all the snort attributes - // LATER add the tag keyword in the rules to capture network traffic - // LATER sanitize every $attribute['value'] to not conflict with snort + // LATER nids - test all the snort attributes + // LATER nids - add the tag keyword in the rules to capture network traffic + // LATER nids - sanitize every $attribute['value'] to not conflict with snort case 'ip-dst': $rules[] = sprintf($rule_format, 'ip', // proto @@ -806,7 +804,7 @@ class EventsController extends AppController { ); break; case 'email-subject': - // LATER email-subject rule might not match because of line-wrapping + // LATER nids - email-subject rule might not match because of line-wrapping $rules[] = sprintf($rule_format, 'tcp', // proto '$EXTERNAL_NET', // src_ip @@ -822,7 +820,7 @@ class EventsController extends AppController { ); break; case 'email-attachment': - // LATER email-attachment rule might not match because of line-wrapping + // LATER nids - email-attachment rule might not match because of line-wrapping $rules[] = sprintf($rule_format, 'tcp', // proto '$EXTERNAL_NET', // src_ip @@ -884,7 +882,7 @@ class EventsController extends AppController { break; case 'user-agent': $rules[] = ""; - // TODO write snort user-agent rule + // TODO nids - write snort user-agent rule break; case 'snort': $tmp_rule = $attribute['value']; @@ -926,7 +924,7 @@ class EventsController extends AppController { // finally the rule is cleaned up and can be outputed $rules[] = $tmp_rule; - // TODO test using lots of snort rules. + // LATER nids - test using lots of snort rules. default: break; @@ -977,7 +975,6 @@ class EventsController extends AppController { // // check if the key is valid -> search for users based on key // $this->loadModel('User'); // // no input sanitization necessary, it's done by model -// // TODO do not fetch recursive // $this->User->recursive=0; // $user = $this->User->findByAuthkey($key); // if (empty($user)) { diff --git a/app/Controller/UsersController.php b/app/Controller/UsersController.php index b0ff283c3..85cfc8fe8 100644 --- a/app/Controller/UsersController.php +++ b/app/Controller/UsersController.php @@ -224,7 +224,6 @@ class UsersController extends AppController { public function login() { - // FIXME implement authentication brute-force protection if ($this->Auth->login()) { $this->redirect($this->Auth->redirect()); } else { diff --git a/app/Model/Attribute.php b/app/Model/Attribute.php index 7bb70d41e..e936df6f2 100644 --- a/app/Model/Attribute.php +++ b/app/Model/Attribute.php @@ -33,7 +33,7 @@ class Attribute extends AppModel { 'private' => array('desc' => 'Prevents upload of this single Attribute to other CyDefSIG servers', 'formdesc' => 'Prevents upload of this single Attribute to other CyDefSIG servers.
Used only when the Event is NOT set as Private') ); - // these are definition of possible types + their descriptions and maybe LATER other behaviors + // these are definition of possible types + their descriptions and maybe later other behaviors // e.g. if the attribute should be correlated with others or not public $type_definitions = array( @@ -297,8 +297,8 @@ class Attribute extends AppModel { $filepath = APP."files/".$this->data['Attribute']['event_id']."/".$this->data['Attribute']['id']; $file = new File ($filepath); if($file->exists()) { - if (!$file->delete()) { - $this->Session->setFlash(__('Delete failed. Please report to administrator', true), 'default', array(), 'error'); // TODO change this message. Throw an internal error + if (!$file->delete()) { + throw new InternalErrorException('Delete of file attachment failed. Please report to administrator.'); } } } @@ -517,7 +517,7 @@ class Attribute extends AppModel { // prepare the conditions $conditions = array( 'Attribute.event_id !=' => $attribute['event_id'], -// 'Attribute.type' => $attribute['type'], // LATER also filter on type +// 'Attribute.type' => $attribute['type'], // do not filter on type ); if (empty($attribute['value1'])) // prevent issues with empty fields return null; diff --git a/app/Model/Server.php b/app/Model/Server.php index a7ebad244..994808f66 100644 --- a/app/Model/Server.php +++ b/app/Model/Server.php @@ -17,7 +17,7 @@ class Server extends AppModel { * @var array */ public $validate = array( - 'url' => array( // TODO add extra validation to refuse multiple urls from the same org + 'url' => array( // TODO add extra validation to refuse multiple time the same url from the same org 'url' => array( 'rule' => array('url'), 'message' => 'Please enter a valid base-url.',