3

I try to pass a form which contains other forms (same inside forms, dynamic) , but I have checked that the data which are sent to the 'script handler' (php) are incomplete data. I think somewhere buffer is overwriting or something. Here is the code :

<?php
if(isset($_POST['submit_num']))
{
    $number=$_POST['sky'];

    if($number== 0)
    {
        header('Location: /ceid_coffee/user_order_form.php');
    }
    else
    {   
        $_SESSION['number'] = $number;
        echo '<form action="user_order_form.php" method="POST">'; 
        for($i=0;$i<$number;$i++)
        {
                $item = $_SESSION['item'];

                echo $item;
                $rec_query = "SELECT * FROM ylika";
                $rec_result= mysql_query($rec_query) or die("my eroors");
                while($row_rec = mysql_fetch_array($rec_result))
                {
                    echo '<br>';    
                    echo '<input type="checkbox" name="yliko[][$i]" value='.$row_rec['onoma'].'> '.$row_rec['onoma'].'';//<~~~~this line is form's data
                }
                echo '<br>';


        }
        echo '<input type="submit" name="submit" value="FINAL_ORDER">';

        echo '</form>';
    }
}

?>

And this is the handling script:

<?php
if (isset($_POST['submit']))
{   
    $number= $_SESSION['number'];
    $item = $_SESSION['item'];
    $max_id = "SELECT MAX(id_order) FROM id_of_orders";
    $x=mysql_query($max_id) or die("my eroors");
    $id= mysql_fetch_array($x);
    $xyz = $id['MAX(id_order)'];
    for($i=0;$i<$number;$i++)
    {
        $temp = $_POST['yliko'][$i]; // <~~~~ this line is the form's data
        $temp2 = implode("," , $temp);
        $inserts = ("INSERT INTO orders (order_id,product,ulika) VALUES ('$xyz' , '$item','$temp2')");
        $inc_prod=("UPDATE proion SET Counter = Counter + 1 WHERE proion.onomasia='$item'");
        mysql_query($inserts) or die(mysql_error());
        mysql_query($inc_prod) or die(mysql_error());
    }
}
?>

This line here contains the data of each form , but i have echo them ($temp2) and i saw that they are incomplete. $temp = $_POST['yliko'][$i];

If i select more than 1 checkbox for each item ($i) I get only one value from the checkboxes into the sql. Do you see if I miss something ?

2 Answers2

1

Ok i found the error. I replace this row :

echo '<input type="checkbox" name="yliko[][$i]" value='.$row_rec['onoma'].'> '.$row_rec['onoma'].'';//<~~~~this line is form's data

with this row :

echo '<input type="checkbox" name="yliko['.$i.'][]" value='.$row_rec['onoma'].'> '.$row_rec['onoma'].'';

I do not know how (i'm new to php) but it worked.

-1

You will only get one value for each form because you are assigning the value of $i to each one:

echo '<input type="checkbox" name="yliko[][$i]" value='.  etc.

is your problem line.

Have a look at the HTML that your code produces (ctrl-u in most browsers) and you will see why you get the wrong answer. All your checkboxes need to have unique names.

I would do it by assigning each checkbox a name that relates to the line in the database from which they are drawn eg:

name="checkbox_"'.$row['ylikaprimarykey']."etc.

This will get you up and running fairly quickly. For what it is worth, the ids of your table keys can give attackers information about your site so it is best practice to obfuscate them in some way. There are a number of excellent classes available free on the net that will do this for you.

If you really need to deal with what would have been in each form as a separate chunk of data, you can easily change the checkbox names vis:

name="checkbox_$formnumber_$obfuscatedkeynumber"

then loop through them with nested loops in your handling page.

Robert Seddon-Smith
  • 957
  • 1
  • 8
  • 13
  • "Best practice" to obfuscate field names? Umm...i've never even seen this done, let alone done it myself, and i'd laugh at anyone who did it. IMO, security through obscurity is worse than no security at all; it gives the implementer a false sense of security. In an even semi-decently designed page, your field names tell an attacker nothing more than that a certain form field has a certain meaning -- which is itself evident from looking at the generated page. If they do any more than that, then your script is severely broken, and obfuscation won't fix it. – cHao Aug 11 '13 at 00:11
  • I would disagree (of course!) as it is very difficult to be certain that you have everything right, it does no harm, and probably some good to obfuscate ids. It has the further advantage of making sure that any data sent to a receiving script must be checked for validity - it is extremely difficult to send spoofed data and very easy to ensure data has only been sent from the particular instance of the page you are expecting it from. Further the code is very short and light so there is no noticeable perfomance hit. You are perhaps mistaking key obfuscation with data obfuscation? – Robert Seddon-Smith Aug 11 '13 at 03:58
  • Ok, I will check it within the hour. Security is not an issue, functionality is. – Βασιλης Ιωσηφιδης Aug 11 '13 at 05:04
  • @RobertSeddon-Smith: It sounds to me like you're suggesting to scramble the names of form controls so that they no longer match the names of the columns/tables they represent. If that's not what you mean, then you might want to disambiguate. But the only other interpretation i see being feasible is scrambling the row IDs rather than the column names...and i'm not seeing how either one really helps. And i've never seen either one done. – cHao Aug 11 '13 at 05:29
  • 1
    i should mention that if i delete the identifier $i for 1 form it works good. – Βασιλης Ιωσηφιδης Aug 11 '13 at 05:52