Welcome Guest, Not a member yet? Register   Sign In
Security precautions when updating database row via URL segments
#1

[eluser]Billy Shall[/eluser]
Normally i'll attach an onchange to a select group for quick changes to a row such as:

Code:
<form>
  <label>Update Status</label>
  <select name="update_status" THE-ONCHANGE="this.form.submit();">
   <optgroup label="Active">
    <option value="1">New</option>
    <option value="2">Updated</option>
    <option value="3">Awaiting Response</option>
    <option value="4">Good Standing</option>
   </optgroup>    
  
   <optgroup label="Inactive">
    <option value="-4">Closed/Resolved</option>
   </optgroup>
  </select>
&lt;/form&gt;

However since adopting twitter bootstrap, i've been trying to set this functionality into their nice looking button dropdown menus.

I've come up with a solution using links instead of the post data:

Code:
<div class="btn-group">
  <a class="btn dropdown-toggle" data-toggle="dropdown">Mark As <span class="caret"></span></a>
  <ul class="dropdown-menu">
   <li><a href="controller/change/&lt;?=$row['id'];?&gt;/status_id/4">Live/Good</a></li>
   <li><a href="controller/change/&lt;?=$row['id'];?&gt;/status_id/3">Awaiting Review</a></li>
   <li><a href="controller/change/&lt;?=$row['id'];?&gt;/status_id/2">Pending Content</a></li>
   <li class="divider"></li>
   <li><a href="controller/change/&lt;?=$row['id'];?&gt;/status_id/0">Deleted</a></li>
  </ul>
</div>

The function for updating the database row would be something like:

Code:
function change($id, $field, $value)
{
  $this->db->where('id', $id)->set($field, $value)->update('table');
  redirect('controller');
}

Aside from Filtering/Validating/Escaping the data and obfuscation of the $id into a unique key, am I going in the wrong direction here? Opening up security holes? Is it a better idea to stick to the good ol' form data?

Thank you!
#2

[eluser]Aken[/eluser]
Changing $field to a column that doesn't exist will throw a SQL error, potentially showing me information about your database you might not want me to know. Same could be said for $value in certain situations.

Also, what if I don't have access to change the row belonging to the unique ID? Nothing stopping me from doing it there...

Never trust your users. Do not allow them to control what your query looks like, ever.
#3

[eluser]Billy Shall[/eluser]
Very true, not a good idea to throw the field in there. Users would be logged in so no outside access.

A better example might be:

Code:
function status($id = FALSE, $val = FALSE)
{
  //Check for variables
  if($id === FALSE OR $val === FALSE)    { show_404(); }
  
  //Is a number?
  if(! is_numeric($id) OR ! is_numeric($val))  { show_404(); }
  
  //Is in range?
  if($val < '-4' OR  $val > '4')      { show_404(); }
  
  $this->db->where('id', $id)->set('status_id', $val)->update('table');
  redirect('controller');
}
#4

[eluser]Aken[/eluser]
Just because I'm logged in doesn't mean I have access to everything (in most cases). You'll likely need to take user permissions into account.

You need to do type casting or something on your values. Because I could still break it by using other numeric values of $id and $val. For instance, "+0123.45e6" is a valid numeric string that would screw up $id. "0.35e1" is a valid numeric string that is between -4 and 4 that could screw up $val.

There's a lot you need to take into account when allowing actions through URLs:
- Is the $id format valid, and does it exist?
- Does the user have access to $id?
- Is the $value in a proper format?
- If it isn't, do you want to change it to a proper format, or throw an error?

There's also an impact on search engines if these URLs happen to be publicly accessible - you probably wouldn't want these indexed. So that's something to consider, also.




Theme © iAndrew 2016 - Forum software by © MyBB