3

I am making a query like this:

$b1 = $_REQUEST['code'].'A'; //letter 'A' is concatenated to $_REQUEST['code']
$a = $_REQUEST['num'];
echo $b1.$a;
$sql = "SELECT '".$b1."' FROM student_record1 WHERE id=".$a;
$result = mysql_query($sql);
if(!$result)
{
    echo '<p id="signup">Something went wrong.</p>';
}
else
{
    $str = $row[0]
    echo $str;
}

Here $b1 and $a are getting values from another page. The 'echo' in the third line is giving a correct result. And I am not getting any error in SQL. Instead, I am not getting any result from the SQL query. I mean echo at the last line.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
yaswanth chilukuri
  • 83
  • 2
  • 5

2 Answers2

3

Don't do this, it breaks your relational model and is unsafe.

Instead of having a table with columns ID, columnA, columnB, columnC, columnD, columnE and having the user select A/B/C/D/E which then picks the column, have a table with three columns ID, TYPE, column and have TYPE be A/B/C/D/E. This also makes it easier to add F/G/H/I afterwards without modifying the table.

Secondly, with the extra column approach you don't have to build your SQL from input values like that. You can use prepared statements, and be safe from SQL Injection. Building SQL from unfiltered strings is wrong, and very dangerous. It will get your site hacked.

Konerak
  • 39,272
  • 12
  • 98
  • 118
  • A rare guest on this site - a sensible answer. – Your Common Sense Nov 03 '11 at 05:41
  • A much more robust solution. +1 to you sir or madam – Phil Nov 03 '11 at 05:46
  • okk... thank u... actually i am not greately aware of this sql injections... can u suggest me some good source to steady them... – yaswanth chilukuri Nov 03 '11 at 05:52
  • @yaswanthchilukuri: You should start by reading what SQL Injection is, [Wikipedia](http://en.wikipedia.org/wiki/SQL_injection) is good enough for that. Only after you understand how the vulnerability manifests, you can start protecting yourself from it. Use the libraries that exist, don't invent your own (it will fail). Since you seem to be using PHP, you will be using [`mysql_real_escape_string`](http://be.php.net/manual/en/function.mysql-real-escape-string.php) or [prepared statements](http://php.net/manual/en/pdo.prepared-statements.php). – Konerak Nov 03 '11 at 07:39
  • @Col.Shrapnel: Thanks, that means a lot, coming from you. – Konerak Nov 03 '11 at 07:41
  • a funny enough, no mysql_real_escape_string nor prepared statements could help with field names – Your Common Sense Nov 03 '11 at 08:15
  • @yaswanth I've covered the whole matter in these answers: http://stackoverflow.com/questions/7537377/how-to-include-a-php-variable-inside-a-mysql-insert-statement and http://stackoverflow.com/questions/7537377/how-to-include-a-php-variable-inside-a-mysql-insert-statement – Your Common Sense Nov 03 '11 at 08:18
  • 1
    I agree it does not help with field names, but since I don't think the field names solution is a good one, I proposed an alternative. If you must use field names, a whitelist is the best option. – Konerak Nov 03 '11 at 13:07
1

If you must use dynamic table/column/database names, you'll have to run them through a whitelist.

The following code will do:

$allowed_column = array('col1', 'col2'); 
$col = $_POST['col']; 
if (in_array($col, $allowed_column)) { 
    $query = "SELECT `$col` FROM table1 "; 
}

See: How to prevent SQL injection with dynamic tablenames?

For more details.

Community
  • 1
  • 1
Johan
  • 74,508
  • 24
  • 191
  • 319