-8

It is a simple PHP MySQL search engine, in which the searching element is "adm_no".

<?php 
require_once("lib/connection.php"); 
require_once("lib/functions.php"); 
$adm_no=$_POST['adm_no'];
//if (!$adm_no==ctype_digit) echo "You Entered wrong Admission no Recheack Admission no" ; exit(); 
$clas=$_POST['clas'];
$query="SELECT * FROM $clas WHERE adm_no = $adm_no";
$result = mysql_query($query);
//searchs the query in db.
while($result = mysql_fetch_array( $result)) 
{ 
echo $result['adm_no']; 
echo " "; 
echo $result['adm_dt']; 
echo ""; 
echo $result['name']; 
echo ""; 
echo $result['dob']; 
echo " "; 
echo $result['f_name']; 
echo " "; 
echo $result['f_office']; 
echo " "; 
echo $result['f_o_no']; 
echo " "; 
echo $result['m_name']; 
echo " "; 
echo $result['m_office']; 
echo " "; 
echo $result['addr']; 
echo " "; 
} ;

And the error I am getting is

Warning: mysql_fetch_array() expects parameter 1 to be resource, array given in C:\wamp\www\st_db_1\search_db.php on line 10

Dharman
  • 30,962
  • 25
  • 85
  • 135
RAVI
  • 11
  • 1
  • 2
  • We're getting fed up with people writing sql injection-vulnerable code too. – Marc B May 18 '11 at 19:03
  • this is really bad code. The SQL injection hole in this code cannot be fixed by using `mysql_real_escape_string()` because you are injecting the tablename into the query, see: http://stackoverflow.com/questions/332365/xkcd-sql-injection-please-explain And then read this question: http://stackoverflow.com/questions/5811834/why-would-this-be-poor-php-code – Johan May 18 '11 at 21:03

3 Answers3

9

You do realize that while($result = mysql_fetch_array( $result)) stomps on the existing value of $result, right? Use a different variable.

Ignacio Vazquez-Abrams
  • 776,304
  • 153
  • 1,341
  • 1,358
  • thank's dude you just solved it but ...when i use $result twice it should flush first $result – RAVI May 18 '11 at 18:57
  • $result is your resource defined outside the loop, and you're overriding it with an array in the loop, therefore it is no longer a resource. – Lotus Notes May 18 '11 at 19:00
  • @Ravi: when you set $result from mysql_query, it's a mysql statement handle. Then you overwrite it in the loop with an array via mysql_fetch. The next time around, it's not longer a statement handle for the fetch call, which is where your error/warning comes from. – Marc B May 18 '11 at 19:04
4

Three issues:

  1. Where are you calling mysql_connect()?
  2. You're overwriting $result in your loop statement.
  3. There's a huge SQL-injection hole in your code.
Johan
  • 74,508
  • 24
  • 191
  • 319
AJ.
  • 27,586
  • 18
  • 84
  • 94
0

RAVI you can fix the SQL-injection code by changing this code:

enter image description hereCoding horror 

$adm_no=$_POST['adm_no'];
$clas=$_POST['clas'];
$query="SELECT * FROM $clas WHERE adm_no = $adm_no";

to this approved code:

$allowed_tables = array('table1', 'table2'); //list of allowed tables
$clas = $_POST['clas'];
$adm_no= mysql_real_escape_string($_POST['adm_no']);
if (in_array($clas, $allowed_tables)) {
    $query="SELECT * FROM `$clas` WHERE adm_no = '$adm_no'";
//                        ^     ^ backticks      ^       ^ single quotes
}

Don't forget to add the backticks ` around tablenames.
And even more important don't forget the single quotes ' around parameters or you will still be vulnerable!

See: How does the SQL injection from the "Bobby Tables" XKCD comic work?
For SQL-injection in general.
And: How to prevent SQL injection with dynamic tablenames?
For SQL-injection problems when using dynamic table or field names.

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