Which syntax is correct from my code?

Question

I am using ajax to search from mysql database.

But I am getting error in query syntax saying error near where division=UNKNOWN.

what is correct syntax

code :

<?php
 include('db.php');

 if(isset($_POST['division'])){
  $division=$database->filter($_POST['division']);
   $check_user = array(
     'division' => $division 
     );
   $exists = $database->exists( 'tablename', 'division', $check_user );

   if ($exists){    
         $sql2 = "select * from tablename where division = '".$division."' group by branch order by branch ASC";

        $sql=$database->get_results($sql2);
        echo '<option value="">--Select Branch--</option>';
         foreach($sql as $row){
             $name=$row['branch'];
             echo '<option value="'.$name.'">'.$name.'</option>';
             }
    }
  }
 ?>

Here Which is correct?

1) 
$sql2 = "select * from tablename where division = '".$division."' group by branch order by branch ASC";

Or

2)
$sql2 = "select * from tablename where division = '$division' group by branch order by branch ASC";

what is the data type of the "division" field in your database? However, you should re-write this anyway because your code is vulnerable to **SQL Injection** attacks. You should use parameterised queries / prepared statements. This should also remove the potential for syntax errors relating to the use, or otherwise, of single quotes round values. — ADyson, Apr 19 '17 at 13:58
No one!!! Use [prepare statement](http://php.net/manual/en/pdo.prepare.php) — Saty, Apr 19 '17 at 13:59
please use `error_reporting(E_ALL); ini_set('display_errors', 1);` on top of your pages and let us know what error occurs. make sure that `division` is set before use -> echo it and see... — OldPadawan, Apr 19 '17 at 14:01
@ADyson It is varchar. secondly I am using mysqli class and using filter to sanitize input. thats why $division=$database->filter($_POST['division']) is used... — Smita Joshi, Apr 19 '17 at 14:01
there's no "filter" method in the main PHP mysqli libraries AFAIK. Is it some sort of plugin or 3rd-party library? IMHO there is no reason not to still use parameterised queries as extra protection. You can't be sure that this filter method is effective against all possibilities. — ADyson, Apr 19 '17 at 14:04
@ADyson Yes...it is mysqli class... here to sanitize data === public function filter( $data ) { if( !is_array( $data ) ) { $data = $this->link->real_escape_string( $data ); $data = trim( htmlentities( $data, ENT_QUOTES, 'UTF-8', false ) ); } else { //Self call function to sanitize array data $data = array_map( array( $this, 'filter' ), $data ); } return $data; } is used — Smita Joshi, Apr 19 '17 at 14:06
"Yes...it is mysqli class". No, it isn't. http://php.net/manual/en/book.mysqli.php . There is no "filter" method mentioned. What library is it? I see it eventually calls mysqli::real_escape_string. As a side effect, that method prevents some attacks by escaping single quotes etc, but that's not its purpose and you can't rely on it. It's just a syntax related function, not security related. See http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string - it refers to the older mysql_ libraries, but the mysqli_ one does almost the same thing. — ADyson, Apr 19 '17 at 14:14
See also http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php/8255054#8255054 — ADyson, Apr 19 '17 at 14:20
I wonder if `where division=UNKNOWN` error message can be linked to `$_POST['division']` being empty though... — OldPadawan, Apr 19 '17 at 14:20
@ADyson I am using mysqli database class available on http://www.phpdevtips.com — Smita Joshi, Apr 19 '17 at 14:21
I see. The discussion of data sanitisation in the docs at http://www.phpdevtips.com/2013/02/custom-php-mysqli-database-class/ appears to relate mainly to encoding HTML entities and dealing with extra slashes. No discussion of SQL Injection, so it's a mistake to assume that it takes care of that aspect. The writer also makes the error of assuming that only user-generated data is a problem. All variable data that gets introduced into a SQL query should be assumed risky. The source of it is not important, the destination (your database) is. Personally I think this class is inadequate. YMMV. — ADyson, Apr 19 '17 at 14:42

score 1 · Answer 1 · edited May 23 '17 at 11:47

As stated in many comments (Joshua Bakker / Saty / ADyson), you should really consider using PPS : Prepared Parameterized Statements. This will help Preventing SQL injection

This is a raw example of what you could use (please adapt to what you need) :

<?php

error_reporting(E_ALL); ini_set('display_errors', 1); /* let PHP help us */

$host = ""; /* your credentials here */
$user = ""; /* your credentials here */
$pwd = ""; /* your credentials here */
$db = ""; /* your credentials here */

/* store in PHP variable */
/* you may also want to perfom some other/more checking on this var */
/* NEVER trust user side data */
$division = $_POST['division'];

echo"[ division -> $division ]"; /* just checking value -> to be removed */

/* connexion to db */
$mysqli = mysqli_connect("$host", "$user", "$pwd", "$db");

if (mysqli_connect_errno()) { echo "Error: no connexion allowed : " . mysqli_connect_error($mysqli); }

/* make sure 'tablename' and 'branch' use below are correct -> adapt to your needs */
$query = " SELECT `branch` FROM `tablename` WHERE division=? GROUP BY `branch` ORDER BY `branch` ASC ";

$stmt = $mysqli->prepare($query); /* prepare query */

$stmt->bind_param("s", $division); /* bind param will sanitize : 
here we make use of $var 'division' with 's' because it's a string AFAIK */

print_r($stmt->error_list); /* any error ? */
print_r($stmt->get_warnings()); /* any error ? */
print_r($stmt->error); /* any error ? */

$results = $stmt->execute();
$stmt->bind_result($branch); /* we use the result of the query */
$stmt->store_result();

if ($stmt->num_rows > 0) {

echo '<option value="">--Select Branch--</option>';

while($stmt->fetch()){

echo '<option value="'.$branch.'">'.$branch.'</option>';
}
}
else
{ echo"[ no data ]"; }
?>

Which syntax is correct from my code?

1 Answers1