2
\$\begingroup\$
$a=$_GET["a"];
$b=$_GET["b"];
$a=str_replace("%", "\%", $a);
$b=str_replace("%", "\%", $b);
$sql="SELECT * FROM table ";
$sql.="WHERE ColA LIKE :txtA AND ColB LIKE :txtB";
$query = $db->prepare($sql);
$query->bindValue(':txtA', '%'.$a.'%', PDO::PARAM_STR);
$query->bindValue(':txtB', '%'.$b.'%', PDO::PARAM_STR);
$query->execute();

I want to give user ability to search anything. Should I filter more characters? Will it be 100% safe and work as it should do for searching pieces of text in database?

Edit: ColA and ColB are strings, just text (phrase and tags in search engine)

\$\endgroup\$

closed as off-topic by Toby Speight, IEatBagels, Stephen Rauch, Donald.McLean, Dannnno Sep 25 '18 at 20:18

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Toby Speight, Stephen Rauch, Donald.McLean, Dannnno
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Please edit to add some description. What do ColA and ColB represent? What's the schema? What flavour of SQL? This mostly looks like stub/sample code rather than real code from a project. \$\endgroup\$ – Toby Speight Sep 25 '18 at 15:13
  • \$\begingroup\$ @TobySpeight edited :) \$\endgroup\$ – Steve143 Sep 25 '18 at 15:19
1
\$\begingroup\$

Security-wise your code is OK. There is not a single variable put inside your query string - so you can tell that it's 100% safe.

Regarding the search quality, like you rightfully noted in the comment, there is also a "_" wildcard character.

Besides, as it is noted in the mysql manual, a backslash character must be escaped with three slashes as well.

So for the bullet-proof escaping for LIKE you could come up with a function

function escapeForLike($str) {
    $str = str_replace("\\", "\\\\\\\\", $str);
    $str = str_replace("%", "\%", $str);
    $str = str_replace("_", "\_", $str);
    return $str;
}

Using this function and some improvements that PDO offers, you can make your code like this

$a = escapeForLike($_GET["a"]);
$b = escapeForLike($_GET["b"]);
$sql="SELECT * FROM table WHERE ColA LIKE :txtA AND ColB LIKE :txtB";
$query = $db->prepare($sql);
$query->execute(['txtA' => "%$a%", 'txtB' => "%$b%"]);
\$\endgroup\$
1
\$\begingroup\$

I want to give user ability to search anything.

It looks like you've achieved that, however, I'd use $_POST to reduce the likelihood that your application will be taken offline by users opening multiple tabs or search engine spiders hitting URLs.

Will it be 100% safe and work as it should do for searching pieces of text in database?

Looks good for SQL injection.

As to whether it "should do" - have you considered generating the query dynamically to allow for tokenized search on inputs like "foo bar"? (i.e. ColB LIKE :token1 OR ColB LIKE :token2)

\$\endgroup\$
  • \$\begingroup\$ I forgot about "_" - if user put that it can change searching a lot, but that should be everything. I wanted use GET to allow users save links to their searches. As we talking about tokens - i am not a good programmer and i have no idea what that is. Thank you very much for answer. \$\endgroup\$ – Steve143 Sep 25 '18 at 14:59
  • 1
    \$\begingroup\$ @Steve143 Tokenization can be as simple as an explode(' ', $input), allowing your users to perform a search on multiple words without requiring that those words appear as an exact phrase. \$\endgroup\$ – Gray Sep 25 '18 at 16:37

Not the answer you're looking for? Browse other questions tagged or ask your own question.