用于安全删除数据库条目的我的Codeigniter方法

Question

I've been searching about deleting db entries in Codeigniter and I finally created a solution that I think is secure. I would really appreciate any feedback! I'm not sure if I'm doing this right..

Advantages:

• Uses POST request

• ID of entry to be deleted is
  validated

• Uses CSRF protection (automatically
  generated by Codeigniter)

In my example I'm deleting user submitted links (a DB table row contains a link title, link URL, an link description).

HTML: Database entires are contained within a form. Each entry has a form button with the respective link id in the id attribute.

<?php echo form_open('profile/remove_link'); ?>

<?php echo form_hidden('link_id', ''); //value will be populated via jquery ?>

<ul id="user_links">
    <?php foreach($query as $row): ?>

    <li><?php echo $row->link_title; ?></li>
    <li><?php echo auto_link($row->link_url, 'url', TRUE); ?></li>
    <li><?php echo $row->link_description; ?></li>

    <button type="submit" class="remove" id="<?php echo $row->link_id ?>"  value="remove">Remove Link</button>

    <?php endforeach; ?>
</ul>

</form>

JQUERY: When user clicks on the remove button, the respective link id is added to the the hidden text input named link_id.

$(document).ready(function(){
    $('.remove').click(function() {
        var link_to_remove = $(this).attr("id");
        $("input[name=link_id]").val(link_to_remove);
    }); 
}); 

Upon clicking a remove button, it sends the id of link to be removed to controller profile and function remove_link

    function remove_link()
    {
        $this->load->model('Profile_model');
        $links_data['query'] = $this->Profile_model->links_read(); //get links from db to add in view

        //Validation
        $this->form_validation->set_rules('link_id', 'Link ID', 'trim|required|xss_clean|max_length[11]|numeric'); //validate link id

        if ($this->form_validation->run() == FALSE) //if validation rules fail
        {           
            $this->load->view('profile/edit_links_view', $links_data);
        }
        else //success
        {
            $link_id =  $this->input->post('link_id'); //get id of link to be deleted
            $seg = 'user_links'; //used to redirect back to user links page
            $this->Profile_model->links_delete($link_id, $seg); //send link id to model function            
        }       
    }

MODEL

    function links_delete($link_id, $seg)
    {
        $this->db->where('user_id', $this->tank_auth->get_user_id());
        $this->db->where('link_id', $link_id);
        $this->db->delete('user_links'); 
        redirect("/profile/$seg/");         
    }

Answer 1

If the ids are unique integers in your database, you could remove these rules:

trim|xss_clean|numeric

And add this one:

is_natural_no_zero

Returns FALSE if the form element contains anything other than a natural number, but not zero: 1, 2, 3, etc.

The numeric rule allows some characters you probably don't want, like decimals and negative. Here's the source (one line):

return (bool)preg_match( '/^[\-+]?[0-9]*\.?[0-9]+$/', $str);

If for some reason you are echo'ing the input back in your HTML output before validating, or are just paranoid, then by all means: xss_clean it up. Otherwise it's not really needed, as I don't think there's any possible method of XSS attacks that only use a number.

Reference:

• https://www.owasp.org/index.php/Cross-site_Scripting_%28XSS%29
• http://ha.ckers.org/xss.html

Also, you might want to add a LIMIT 1 clause to your query, and definitely make sure to return a value (probably TRUE/FALSE) from your model so you know whether or not the query was successful, so you can give feedback to the user instead of assuming everything went well.