+- CodeIgniter Forums (https://forum.codeigniter.com)
+-- Forum: Using CodeIgniter (https://forum.codeigniter.com/forumdisplay.php?fid=5)
+--- Forum: General Help (https://forum.codeigniter.com/forumdisplay.php?fid=24)
+--- Thread: Have I made form function secure enough (/showthread.php?tid=64852)
Have I made form function secure enough - wolfgang1983 - 04-02-2016
I would like to know if I have made my form function secure enough.
So if user try's to access form with out a id it redirects etc but also trying make it protected as well.
I have not enabled CSRF should I?
PHP Code:
<?php
class Banners extends CI_Controller {
public $error = array();
public function __construct() {
parent::__construct();
}
public function edit() {
if ($this->input->server('REQUEST_METHOD') == 'POST') {
}
$this->get_form_edit();
}
public function index() {
$this->get_list();
}
protected function get_list() {
$data['template'] = 'template/banners/banner_list_view';
$this->load->view('template/common/template_view', $data);
}
protected function get_form_edit() {
$banner_form_id = isset($_GET['banner_form_id']) ? $_GET['banner_form_id'] : "";
if ($banner_form_id) {
$data['banner_form_id'] = $banner_form_id;
$data['placeholder'] = 'holder.js/100%x75';
$data['template'] = 'template/banners/banner_form_edit_view';
$this->load->view('template/common/template_view', $data);
} else {
redirect('c=banners');
}
}
}
Is there any thing need to add in view?
PHP Code:
<div class="container">
<div class="row">
<div class="col-lg-3 col-md-3 col-sm-12 col-xs-12">
</div>
<div class="col-lg-9 col-md-9 col-sm-12 col-xs-12">
<?php echo form_open_multipart('c=banners&f=edit&banner_form_id=' . $banner_form_id);?>
<table id="banners-table" class="table">
<thead></thead>
<tbody></tbody>
</table>
<?php echo form_close();?>
</div>
</div>
</div>
RE: Have I made form function secure enough - albertleao - 04-02-2016
Hello Wolfgang!
I would recommend doing checks and routing for method types in your routes.php. The manual will show you how you can assign the different HTTP methods to different routes.
Otherwise, I don't see any benefit of creating protected routes like you are doing. There shouldn't be any benefit of calling $this->get_list from your index function when compared to simply loading the view from your index function. One thing that a lot of programmers make a mistake on is they think that by making things more complicated makes the application more secure. This cannot be further from the truth.
I'm not sure what type of security you are looking for, but I'd recommend setting up your security somewhere else. Using an Auth library or creating a function in your MY_Controller that you can call to verify your users might be a place to start. But I do warn against building your own Auth library if you are not experienced in it. Authentication is not a piece you want to "build as you learn"
RE: Have I made form function secure enough - Diederik - 04-02-2016
You are not validating the value of $_GET['banner_form_id'], you only check if it exists. Based on the name of the variable I would guess it would represents an integer based id. Eventhough GET variables are filtered by the config (default value: $config['permitted_uri_chars'] = 'a-z0-9~%.:_\-'
and your code does not represents any harmful processing (although a bit over-complexing solution to a simple situation).I would suggest some small modification like:
PHP Code:
$banner_form_id = intval( $this->input->get('banner_form_id'));
About your CSRF question. Genaraly there are few reasons why you should not use it although the impact of a succesfull CSRF attack depends on what functionality the form has and the role of the current user.
For more details I would refer you to: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prevention_Cheat_Sheet
RE: Have I made form function secure enough - Tpojka - 04-03-2016
If not used conversion base other than 10 (which in this scenario would be a case), (int) is faster than (intval).