1

I am creating a website using PHP PDO oops concept. For like - I want to count columns for different columns. I have created a function in a class. I follow all steps to secure data (SQL injection). My function -

public function count_by_id($table,$col,$val)
{ 
 $table=$this->sanitize($table,'string');
 $col=$this->sanitize($col,'string');
$val=$this->sanitize($val,'string');
 $sql= "SELECT count(*) FROM $table WHERE $col=:val"; 
     $stmt = $this->dbConnection->prepare($sql);
     $stmt->bindParam(':val', $val, PDO::PARAM_STR);
    $stmt->execute();
        $number_of_rows = $stmt->fetchColumn(); 
        return $number_of_rows;  
}

$table is a static variable that will not change any value. I use it only for table name and also the same for column name. The table and col values will not be changed by the end user. The end user will change only $val value and I have bound that value using a prepared statement.

Like - Calling a function -

  count_by_id('users','username',$username);

The users and username will not change but $username will change and I have bound it. Is there any reason for SQL injection or not? I am not getting the table name and column name from the form. I can use it for different table and column like this-

 count_by_id('posts','slug',$slug);

I am totally confused because many programmers are doing like me and many say this may be the reason for SQL injection. What is your view on that ?

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
Vishal Rana
  • 11
  • 2
  • Man I offered you a perfect solution to this problem. Why don't you just use it? I even bothered to [write this function for you](https://phpize.online/sql/mysql57/undefined/php/php81/0bd967f1ba125b1d10f8372c12090206/). See the `Find by column: found`? – Your Common Sense Feb 15 '23 at 06:13
  • I have already checked both questions but my question is different. Kindly remove duplicate question tag . – Vishal Rana Feb 15 '23 at 06:14
  • 1
    Your question is **nothing new**. I told you already, adding an unprotected variable directly to SQL string is what's called SQL injection. And not a single programmer in would tell you otherwise. Your sanitize function is a **nonsense** that protects nothing, but only corrupts your data. So do the duplicate answers say – Your Common Sense Feb 15 '23 at 06:19
  • @YourCommonSense As per your solution, we can set the table name in child class, and what about a different column name? For that, we will create the same function again for the different column in the same class. – Vishal Rana Feb 15 '23 at 06:22
  • You didn't check the code, did you? We are setting the column names AS WELL. Because you will need them for INSERT and UPDATE functions AS WELL, and you will write them INEVITABLY. It's on the 4th line. Is it that hard to spot? – Your Common Sense Feb 15 '23 at 06:24
  • ok , Here ` protected $primary = 'id';` column name is already set to id but if I want to use $column='username'; or $column='email'; then that means I have to create another protected variable and validate with fields array , Right ? – Vishal Rana Feb 15 '23 at 06:29
  • I posted a link. In the first comment here. Would you like to follow this link and check the 4th line in the code, please? – Your Common Sense Feb 15 '23 at 06:35
  • Then, would you like to check the line 36 and tell me, what's so wrong with a function getByField() that you cannot use it for your purpose? – Your Common Sense Feb 15 '23 at 06:49
  • There, `SELECT * FROM `$this->table` WHERE `$fieldName`=?` we are giving tablename and fieldname direct in query. Is it fully safe? – Vishal Rana Feb 15 '23 at 07:05
  • 1
    As long as you are sure that `$this->table` and `$fieldName` can only ever be values that _you_ control and can never accidentally be user input: yes. Thus see the second duplicate linked above: https://stackoverflow.com/q/182287/476 – deceze Feb 15 '23 at 07:24
  • Would you like to check the line 44 where it uses `; drop table users` as the column name? And then press the `Run PHP code` and check the output that says `Fatal error: Unknown field(s): ; drop table users`? – Your Common Sense Feb 15 '23 at 07:25
  • @deceze that's not enough. The key is, *where* this control occurs. He would say that the way this function is *called* it's safe, as he hardcoded it: `count_by_id('users','username',$username);` . But it only makes safe **one particular call** but **not the function itself**. An it's the very difference between safety and injection. – Your Common Sense Feb 15 '23 at 07:33
  • @YourCommonSense Yes , I ve got a Fatal error:Unknow field. You mean, we need to create an array and validate the column with that array element then execute the function. – Vishal Rana Feb 15 '23 at 07:56
  • Yes, that's what I mean exactly. You got to write the table and the column names only once but then your code will become shorter and safer. the code to validate is already written. – Your Common Sense Feb 15 '23 at 07:57
  • 1
    Ok, thank you @YourCommonSense. I will definitely try this type of structure. – Vishal Rana Feb 15 '23 at 08:00

0 Answers0