i have array variable for textboxes how can i use $_POST correctly?

Question

Registration

<?php
session_start();
$connection=Mysql_connect('localhost','admin','123');
Mysql_select_db('db',$connection);
if(array_key_exists('insert',$_POST))
{
$query="select * from pharmacy";
$result=mysql_query($query);
if (!$result)
{
print(mysql_errno() .":". mysql_error());
}
$num=Mysql_num_rows($result);
$num1=Mysql_num_fields($result);
    if($num>0)
    {
    echo "<table border=2>";
    for($i=0;$i<$num;$i++)
    {
    $row=mysql_fetch_row($result);
    echo "<tr>";
    echo"<td><input type='Checkbox' name='p[$i]'  value='on' unchecked /></td>";
    echo"<td>$row[0]</td>";
    echo"<td><input type='txt' name='q[$i]' /></td>";
    $r[$i]=$row[0];
    if(isset($_POST['q']))
    $q[$i]=$_POST['q'];
    echo"</tr>";
    }//for
    echo"</table>";
    }
    if(isset($_POST['p']))
    foreach($_POST['p'] as $key=>$value)
        {
        if($value=="on")
        {
       $u=$_SESSION['t'];

       $query8="insert into $u(name,qun)values('$r[$key]',$q[$key])";
      echo $query8;
       $result8 = mysql_query($query8);
    //header("Location: show.php?");
    }
    echo $q[0];
       }//for

    }
?>
<input type="submit" name='insert' value="insert Drugs"/>
</form>
</body>

i have a table that has rows i insert the chosen ones in another table in mysql but when i want to insert the content of texts i have problem my problem is here:if(isset($_POST['q'])) $q[$i]=$_POST['q']; it can't be set how can i correct it?

Well, someone is on a downvoting streak, an explanation would be nice... — jeroen, May 31 '11 at 15:49
@jeroen & all, got called away, sorry about that. The problem with dynamic table and column names is that there is no way to secure them **other** than using a whitelist of table and column names check against those and only allow values that match. This query not only injects values, but injects table names as well. If I fill in `databaseX.tableY` I can insert any value into any database on the server. Heck I can even inject into the `mysql` database and grant myself privileges. It shouldn't be **this** easy. If the question is `how can i use $_Post correctly` you expect this issue to come up — Johan, May 31 '11 at 16:25

score 3 · Answer 1 · edited May 23 '17 at 10:32

This code:

enter image description here coding horror

$query8="insert into $u(name,qun)values('$r[$key]',$q[$key])";

Is an injection nightmare!

There is so much wrong with this code from a security point of view:

Always use $var = mysql_real_escape_string($_POST['var'');
Always surround your $vars used for values in a query with ' single quotes.
If you use dynamic database, table or fieldnames mysql_real_escape_string will not work nor will any other escape function.
You will need to check all table names and field names against a list of pre-approved table and field names.
If you must use dynamic field and/or column names, escape them with ```; this is not for security but to prevent syntax errors in your query when using reserved words or numbers as column/table names.

See this question for more details: How to prevent SQL injection with dynamic tablenames?

you mean first i must write $var = mysql_real_escape_string($_POST['var']) and then $query8="insert into $u(name,qun)values('$r[$key]',$q[$key])";? — Nickool, May 31 '11 at 15:53
That's step 1 You have now secured 1% do steps 2 to 5 do close the other 99% of your SQL-injection hole. — Johan, May 31 '11 at 15:55

jeroen · Answer 2 · 2011-05-31T15:45:30.880

-1

$_POST['q'] is an array, so your $query8 will fail as you use:

$q[$i]=$_POST['q'];

All values of $q are arrays and you can´t insert an array in a database like this:

$query8="insert into $u(name,qun)values('$r[$key]',$q[$key])";

You probably need something like:

$q[$i]=$_POST['q'][$i];

Edit: By the way, you always need to prepare your data for use in a database. I prefer prepared statements / PDO but if you use regular mysql you need to escape your variables before you insert them using something like mysql_real_escape_string.

Edit 2: In case of variable table or column names, always check them against a white-list.

edited May 31 '11 at 15:45

answered May 31 '11 at 15:21

jeroen

91,079
21
114
132

@jeroen, -1, have you never heard of SQL-injection? – Johan May 31 '11 at 15:26
@Johan I keep forgetting the `mysql_real_etc_etc_etc` stuff as I always use prepared statements and that code took all my concentration :( – jeroen May 31 '11 at 15:29
Johan i know SQL-injection but i don't know why you suppose that this solution is not good? – Nickool May 31 '11 at 15:33
@Negin Never trust sent in data like `$_POST`, always use `mysql_real_escape_string` on the individual variables before inserting them in the database. – jeroen May 31 '11 at 15:36
@jeroen, PDO will not work here, this code is injectable even with PDO! – Johan May 31 '11 at 15:38
@Negin, it's an injection nightmare, worst code I've seen to date on SO. – Johan May 31 '11 at 15:39
@Johan Using a prepared statement?! How? – jeroen May 31 '11 at 15:39
1

@Johan Ah, I see he´s using a variable as a table name, didn´t notice that. @Negin, in case of a variable table name, always check against a white-list of allowed table names. – jeroen May 31 '11 at 15:41

score -1 · Answer 3 · answered May 31 '11 at 15:22

You really should separate the form handling code from the form generation code. Such a hideous mix is hard to debug.

<?php

if ($_SERVER['REQUEST_METHOD'] == 'POST') {
    if (isset($_POST['p']) && is_array($_POST['p'])) {
        foreach($_POST['p'] as $key => $val) {
           ... do db stuff ...
        }
    }
}

... generate form here ...

i have array variable for textboxes how can i use $_POST correctly?

3 Answers3