missing elements of $_POST array

[Mild Fuzz]

hello

trying to add a mass-delete function a WP-plugin, which works fine if you want to delete 1,2,3 or 4 entries, but anything higher is lost.

Perversely, it doesn't even take the missing elements from with end of the array, it takes the from the middle!!

Grr, help!!

This is the function that writes the table:

function mp_timetable_results(){
    global $wpdb;
    $table_name = $wpdb->prefix . "mf_timetable";
    $query="SELECT * FROM " . $table_name;
    $results = $wpdb->get_results($query);
    ?><form name="timetable_delete" method="post" action="<?php echo str_replace( '%7E', '~', $_SERVER['REQUEST_URI']); ?>"><input type="hidden" name="mf_timetable_delete_hidden" value="Y"><table class="mf_timetable_admin"><?php
    foreach ($results as $results){
        ?><tr><?php
        ?><td><?php
        echo covert_day_output($results->day);
        ?></td><?php
        ?><td><?php
        echo $results->class;
        ?></td><?php
        ?><td><?php
        echo $results->location;
        ?></td><?php
        ?><td><?php
        echo $results->start;
        ?></td><?php
        ?><td><?php
        echo $results->end;
        ?></td><?php
        ?><td><?php
        echo $results->teacher;
        ?></td><?php
        ?><td>£<?php
        echo $results->cost;
        ?></td><?php
        ?><td><?php
        echo $results->category;
        ?></td><td><input type="checkbox" name="<?php echo $results->class; ?>" value="<?php echo $results->id; ?>" /></td></tr><?php
    }
    ?></table><input type="submit" name="delete_selected" value="<?php _e('delete selected', 'mf_timetable_delete' ) ?>" /></form>
    <?php
}  

and this is the delete function

function mp_timetable_delete(){
    global $wpdb;
    //var_dump($_POST);
    echo implode(",", $_POST);
    $table_name = $wpdb->prefix . "mf_timetable";

    foreach ($_POST as $list){
        
        
        $sql= "DELETE FROM " . $table_name .
               " WHERE id = " . $list;
        $test=$wpdb->query($sql);
        
    };
    
    ?>
    <div class="updated"><p><strong><?php _e('Event Deleted.'); ?></strong></p></div>
    <?php
}  

[chipbennett]

This may sound crazy (and slightly off-topic), but have you considered custom post types in WP 3.0? Would making a custom post type of "dance class" (or "event") make your life easier at all?

[itsananderson]

I'm gonna be brutally honest here, so bear with me. Your code needs a LOT of work.

You're using a different post variable for each request, which I suspect is your problem because any results with the same class name will overwrite each other. You should be using only one post variable and harnessing the automatic array foo (not the technical term).

You're also not following proper WP SQL practices which, among other things, opens a HUGE gaping hole into your DB. If you don't believe me, send me a link to where this code is running and I can blow away your entire database.

OK, enough berating. On to the helping part.

Here's a new version of your code that should perform better and eliminate the problems I mentioned.

 <?php
function mp_timetable_results(){
    global $wpdb;
    $table_name =  . "mf_timetable";
    $query="SELECT * FROM ${wpdb->prefix}mf_timetable";
    $results = $wpdb->get_results($query);
    ?><form name="timetable_delete" method="post" action="<?php echo str_replace( '%7E', '~', $_SERVER['REQUEST_URI']); ?>"><input type="hidden" name="mf_timetable_delete_hidden" value="Y"><table class="mf_timetable_admin"><?php
    foreach ($results as $results){?>
        <tr>
            <td><?php echo covert_day_output($results->day); ?></td>
            <td><?php echo $results->class; ?></td>
            <td><?php echo $results->location; ?></td>
            <td><?php echo $results->start; ?></td>
            <td><?php echo $results->end; ?></td>
            <td><?php echo $results->teacher; ?></td>
            <td>£<?php echo $results->cost; ?></td>
            <td><?php echo $results->category; ?></td>
            <td><input type="checkbox" name="mp_timetable_delete[]" value="<?php echo $results->id; ?>" /></td>
        </tr><?php
    }
    ?></table><input type="submit" name="delete_selected" value="<?php _e('delete selected', 'mf_timetable_delete' ) ?>" /></form>
    <?php
}
?>  

 <?php
function mp_timetable_delete(){
    global $wpdb;
    $table_name =  . "mf_timetable";
 
    foreach ($_POST['mf_timetable_delete'] as $id){
        $sql= $wpdb->prepare("DELETE FROM ${wpdb->prefix}mf_timetable" .
               " WHERE id = %d" , $id);
        $test=$wpdb->query($sql);
    }
    ?>
    <div class="updated"><p><strong><?php _e('Event Deleted.'); ?></strong></p></div>
    <?php
}
?>  

Now I haven't tested this code (how could I) but barring some silly mistake, it should work a whole lot better.

[Mild Fuzz]

Thanks for the feedback.

Could you explain how it could be exploited? I'm new to this so it would be great if I understood my balls ups as best I can.

[itsananderson]

OK, so basically the way you're putting the user input from $_POST into the query without checking it first. This means that a user could submit a variable containing SQL code and it would be executed on your database.

As an example, if I edit your form and set the value of a checkbox to "1 OR 1", the complete query will be "DELETE FROM {table_name} WHERE id = 1 OR 1". This will delete ALL entries in that table, which is NOT what you want.

Furthermore, a more advanced query could be created to delete other tables. e.g. "1; TRUNCATE TABLE wp_users; --" would delete all the users in the database (assuming you're using "wp_" as the database prefix).

In general, you should follow these guidelines when querying databases using user input.

http://codex.wordpress.org/Data_Validation#Database

The rest of that page is also very important, but that section deals specifically with user input and databases.

[Mild Fuzz]

ahh, okay, I get ya! So they could alter the html before sending.

Fortunately, this is only visible in the admin panel, so nothing to worry about too much, but definitely something to bare in mind.

Thanks.