PHP code for search page

Question

Want to show you part of code that I wrote to process search request. User commit search by articles (codes) of products. The goal is to allow user write his search request to textarea element in any possible way: in a row, in a column or separated by any separator (comma, space, semicolon etc.).

<?php
include("db.php"); // Including connection to MySQL
if(!empty($_POST['codes'])){ // Checking user's input from textarea not empty
 $codes = $_POST['codes']; // Moving user's input to variable
 preg_match_all("/\d{6}/", $codes, $codes_array); // Taking all elements that matches regular expression and adding them to the array
 $codes_array = array_unique($codes_array[0]); // Removing duplicates from the array
 if(!empty($codes_array)){ // Checking array not empty
 $codes_list = ""; // Creating new variable
 foreach ($codes_array as $key => $value) { // Processing our array
 $codes_list .= $value . "|"; // Adding values and separator to previously created variable to use it in MySQL query
 }
 $codes_list = rtrim($codes_list, "|"); // Removing last separator from string
 $codes_add = " AND `code` REGEXP '^({$codes_list})$'"; // Creating new variable with part of MySQL query and our elements from the array
 } else {
 $codes_add = ""; // Creating empty variable if our array was empty
 }
}
$query = "SELECT * FROM `table` WHERE `status` = '0'" . $codes_add; // Generating MySQL query string
$result = mysqli_query($link, $query); // Making a request to MySQL
?>

My friend said that it contains vulnerabilities for SQL-injection because I do not filter user's input. But my logic based on a statement that user's input filters at the line with preg_match_all. I'm sure that nothing but elements that consist of 6 digits will be included to the array.

Anyway I want that someone who have an experience with PHP and vulnerabilities took a look at this code and share his opinion about potential risks of using it. Thank You.

• Ron Beyer: I won't comment on the security of this, but you are really over-commenting your code. Not every line needs a comment, code should be self-explanatory, for example if(!empty($codes_array)){ // Checking array not empty it is pretty obvious reading the if statement that it checks for an empty array, what is the comment telling the reader exactly? Comments should be there for extraordinary explanation, not to map out your thought process, it adds a lot of noise to your code.
• BCdotWEB: "Code Tells You How, Comments Tell You Why"
• Wiimm: A note about empty(): The input of "0" is ignored (assumed as empty). For string it is better to use isset($_POST['codes']) && strlen($_POST['codes']) to allow the input of "0".

Answer 1

Being as big an advocate of prepared statements as I am, I cannot find any way for the SQL injection here, so your code should be safe.

I could only do the usual routine of making this code less bloated:

$query = "SELECT * FROM `table` WHERE `status` = '0' ";
if(!empty($_POST['codes'])) {
 preg_match_all("/\d{6}/", $_POST['codes'], $codes_array);
 $codes_array = array_unique($codes_array[0]);
 $codes_list = implode(",", $codes_array);
 $codes_add = $codes_list ? " AND `code` IN ($codes_list)" : "";
 $query .= $codes_add;
}
$result = mysqli_query($link, $query);

basically, PHP already has a function to glue array values together called implode() and, based on my understanding of the SQL, I decided to change REGEXP for IN. If I am wrong, then you could simply change it back to regexp.

• mickmackusa: You might like to position the preg_match_all() call up in the conditional, just in case the user input only has invalid characters.