Welcome Guest, Not a member yet? Register   Sign In
Iterating through a form's $_POST
#1

[eluser]stevefink[/eluser]
Hi all,

So I have a fairly large form. About 8 text fields, 3 different select boxes, one radio group with two choices, some textareas etc.

Some of these fields are required, some are not. I have my validation working perfectly.

My question is, what technique would you folks use to insert the data that is submitted via $this->input->post to take the validated data and insert it into my respective database table? I'm not sure if I should check for every single potential post value, that could take a lot of code.

If anyone can throw an example here, that'd be awesome.

Thanks!
#2

[eluser]Rick Jolly[/eluser]
The validation class works directly on the $_POST array to make it safe for insert into a database. So assuming your database field names are the same as the keys in the $_POST array, just do an active record insert with the $_POST array.
#3

[eluser]Scott - Beyond Coding[/eluser]
[quote author="Rick Jolly" date="1187838835"]The validation class works directly on the $_POST array to make it safe for insert into a database. So assuming your database field names are the same as the keys in the $_POST array, just do an active record insert with the $_POST array.[/quote]

I checked the validation class code and you're right in saying this.. Although is it best practice to do it this way?

I love the simplicity of the suggestion but I always feel a little scared passing $_POST directly to the DB.. What if someone injected a variable that wasn't in the DB.. could that get past the validation and allow an exploit? Just thinking out loud here.. Smile
#4

[eluser]Michael Wales[/eluser]
The vulnerability comes into play when someone injects a variable that is in the database but not in your form.

An example that was previously given on this forum (can't find the thread atm):
If you are updating a record's image and location field via a form, and that table also has a permissions field - the malicious user could inject 'permissions'='admin' and gain administrative privileges to your site.

Always strictly define the input variables via one of these (which all equate to the same value):
Code:
$this->input->post('var');
$this->validation->var;
$_POST['var'];
#5

[eluser]Phil Sturgeon[/eluser]
[quote author="Goo Theory" date="1187865388"]I love the simplicity of the suggestion but I always feel a little scared passing $_POST directly to the DB.. What if someone injected a variable that wasn't in the DB.. could that get past the validation and allow an exploit? Just thinking out loud here.. Smile[/quote]

The auto-xss_clean feature should take care of it as should the AR's adding of slashes but I don't trust such things to work after reading around these forums.

A simple little bit of code to make a clean $_POST is:

Code:
foreach($_POST as $key => $val):
    $data[$key] = $this->input->xss_clean($val);
endforeach;

That may well be redundant but always makes me feel safer! It may well be easier to just do:

Code:
foreach(array_keys($_POST) as $key):
    $data[$key] = $this->input->post($key, TRUE);
endforeach;

not sure if either is faster/better but are both just as safe.
#6

[eluser]Rick Jolly[/eluser]
[quote author="walesmd" date="1187868712"]The vulnerability comes into play when someone injects a variable that is in the database but not in your form.

An example that was previously given on this forum (can't find the thread atm):
If you are updating a record's image and location field via a form, and that table also has a permissions field - the malicious user could inject 'permissions'='admin' and gain administrative privileges to your site.
[/quote]
Good point and I should watch for those cases.

[quote author="walesmd" date="1187868712"]
Always strictly define the input variables via one of these (which all equate to the same value):
Code:
$this->input->post('var');
$this->validation->var;
$_POST['var'];
[/quote]
Always is a strong word. As you mentioned, there is only a vulnerability when a user injects a variable that is in the database but isn't in the form. But when in doubt, the database fields should be defined.

Solution
I think this is a simple safe solution that saves typing:
Code:
// set the validation fields
$fields['address'] = "Address";
$fields['price'] = "Price";

$fields = array_keys($fields);
// OR:
// $this->validation->set_fields($fields);
// $fields = array_keys($this->validation->_fields);

$insert_data = get_db_data($fields, $_POST);
$this->db->insert('mytable', $insert_data);


function get_db_data($array_fields, $array_post)
{
   $data = array();

   foreach ($array_fields as $field)
   {
      $data[$field] = $array_post[$field];
   }
  
   return $data;
}
#7

[eluser]Phil Sturgeon[/eluser]
While I had never heard of that vulnerability, my code has quite a nice way of avoiding it by accident.

All my models contain a property called $fields which is just a list of variable names. It then does a loop like Rick's here but instead of directly inserting a post variable it uses the input class so it will XSS clean and return empty string if its not set.

Code:
<?php
class Category_model extends Model {

var $fields = array('catID', 'cat_name', 'cat_description');

function update($catID)
    {
    
    foreach($this->fields as $field):
        $row[$field]     =   $this->input->post($field, TRUE);
    endforeach;

        $this->db->update('Categories', $row, array('catID' => $catID));
        
    // return true or false based on how many rows returned
    return($this->db->affected_rows() > 0);
        
    }
#8

[eluser]Michael Wales[/eluser]
pyro - I'm not seeing how that would prevent this type of vulnerability. Let's say your Category model had a field called permissions - which defined who was allowed to edit it.

Scenario:
Quote:Your fields array would then include this new permissions field (assuming you had a reason to edit it via the web interface, maybe you have an admin area or something).
The malicious user injects this variable into the $_POST array.
You loop through the fields array, dropping all of the established variables into the row array.
You insert this data into your table - which now includes the permissions field you were not expecting (because it's not in your form, but the user included it).

I do think you are on to something here though (it just needs to be moved to the method level, not the class level) - and to be honest, I never thought of looping through like that - it would save a lot of typing time on my end. I'm going to go back and edit my current project to use this particular method.

But, what if you defined the fields array for each method? Therefore, you are limiting what can/can not be placed within the database. To mimic the scenario I outlined above:
Quote:Your method's field array only contains catID, cat_name, and cat_description - since that's all the user can edit via this particular method.
Malicious user injects his variable.
You loop through all of your editable fields, as defined in this method, and the one he injected gets set to FALSE, NULL, '' - whatever.
All is well in the world.
#9

[eluser]Phil Sturgeon[/eluser]
A bit too drunk to be reading that entire post but to answer your first bit, this stops it because you are defining a list of ALL the fields in your DB. This then means that for EVERY field in the DB they will either have the right value or empty. So while they can insert extra variables into the $_POST array, if they do it will get cleaned.

The rest can wait till tomorrow x_x
#10

[eluser]Scott - Beyond Coding[/eluser]
Michael, pyro, Rick.. thanks for all your replies. It's very interesting to see how others tackle the same problem Smile

I often use a similar approach to pyro although I also specify the type of field and then typecast anything that should be an integer or bool.. Something along the lines of this (keeping with the same example variables):
Code:
$fields = array('numbers' => array('catID'),
            'text' => array('cat_name', 'cat_description'));

foreach($fields['text'] as $field)
    $user_input[$field] = $this->input->post($field, true);

foreach($field['numbers'] as $field)
    $user_input[$field] = (int) $this->input->post($field, true);

I think Rick's idea of setting and checking $fields as per the validation library that could be a much neater solution though (as you can set validation rules to handle numbers appropriately etc). Could also cycle through $rules too provided you've set rules for everything Smile




Theme © iAndrew 2016 - Forum software by © MyBB