0

I'm learning PHP and would like to ask which of these two different approaches is better in terms of SQL server security? Let's say it will be used for a login form.

Function #1

function SanitizeInput($in, $len = 20)
    {
        return substr(preg_replace("/[^a-zA-Z0-9_]/", "", htmlspecialchars(htmlentities(strip_tags($in)))), 0, $len);
    }

Usage:

$username = $thisDB->SanitizeInput($_POST['username']);

Function #2&3

    function security($text) {
    $text = trim($text);
    $search = array('', '', '', '', '', '', '', '', '', '', '', '', ',');
    $replace = array('C', 'c', 'G', 'g', 'i', 'I', 'O', 'o', 'S', 's', 'U', 'u');
    $new_text = str_replace($search, $replace, $text);
    return $new_text;
}
function SQLSecurity($text) {
    $text = trim(htmlspecialchars($text));
    $search = array("'", '"', "TRUNCATE", "truncate", "UPDATE", "update", "SELECT", "select", "DROP", "drop", "DELETE", "delete", "WHERE", "where", "EXEC", "exec", "INSERT INTO", "insert into", "PROCEDURE", "procedure", "--");
    $replace = array("", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "");
    $new_text = str_replace($search, $replace, $text);
    return $new_text;
}

Usage:

$username = $thisDB->SQLSecurity($thisDB->security($_POST['username']));

Korla
  • 19
  • 3
  • 3
    So *many* duplicates. **Use parametrized queries** and don’t allow direct SQL as input. Using this *decades-old secure coding style entirely prevents first-order SQL injection* and mitigates the perceived need for such misguided “sanitization”. – user2864740 Oct 10 '20 at 01:27
  • Does this answer your question? [How can I prevent SQL injection in PHP?](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – user2864740 Oct 10 '20 at 01:28
  • Now, if there are *business rules*, such as restrictions of valid characters in usernames, apply those as such applies. These business rules (information quality) are *outside* of SQL injection concerns (data transfer). If there are second-order SQL injection issues, this is a problem with the the target usage not correctly furthering the use of safe parameterized queries. – user2864740 Oct 10 '20 at 01:32
  • I’m also not sure why this `security` function would be used — inspect the input/output. *Why*? Even if written with the “correct” replacement usages (hint: arrays have wrong data), it *alters* the information. Should a username *really* not be allowed any of those characters? – user2864740 Oct 10 '20 at 01:37
  • I made the first `SanitizeInput` function to replace the other two and still do the same: prevent sql injection. The 2nd and 3rd functions were suggested by a friend of mine which is learning php too, but I strongly believe my first function `SanitizeInput` is better and will do the job. My question is simple: is `SanitizeInput` better than the others? Regarding the parametrized queries, I'll see what I can do, but I'm not that advanced and probably will take some time till I learn more to convert these current ones in to parametrized queries. – Korla Oct 10 '20 at 01:43
  • BTW, with this horrid code, I can’t login with my username of `ExecutorGold` :( – user2864740 Oct 10 '20 at 01:43
  • This “belief” is misguided. Read through the duplicate. If *needing* to use raw values in SQL value positions, use [msqli_real_eacape_string](https://www.php.net/manual/en/mysqli.real-escape-string.php), and take time to understand why such is *sufficient* (even if a poor alternative to *parameterized queries*) when used in SQL that properly contains quotes for string literals. – user2864740 Oct 10 '20 at 01:44
  • @user2864740 - Users should be allowed to use usernames such as `replace` and such and that's why I made the `SanitizeInput` to replace the other two and still prevent any sql injection. – Korla Oct 10 '20 at 01:45
  • @user2864740 - So I can do something like `$username = $thisDB->mysql_real_escape_string($_POST['username']);` which will be better than all presented in my post? Thanks for taking the time to help me out. Isn't this only for MySQL and not MSQL Server – Korla Oct 10 '20 at 01:48
  • If not able to use parameterized queries, yes. However, it would be `$myDB->real_escape_string` with the non-static form. See the link. Then **in usages, ensure that quotes are used correctly in the SQL for all string positions**, or positions that can receive strings: `SELECT * FROM users WHERE name = '$username'` (when ANSI quotes are enabled). It would be *better* to use *parameterized queries*, and to update any code to use such. – user2864740 Oct 10 '20 at 01:51
  • I bolded the above bit, because *only* values already numbers should be used without quotes. Otherwise this could cause injection: `$n = $POST["usually_number"]; $s = real_escape($n); query("select .. where id = $s")`. The only correct way to use a non-quoted value first, is to convert it to an integer or other suitable number, eg: `$n = ..; $s = (int)$n ..`; in this case no “escaping” is needed and $s can/should be left without quotes in the query. – user2864740 Oct 10 '20 at 01:56
  • So, back to the initial proposition/claim: **use placeholders**; then this case no longer exists, and one can clean up SQL literals by removing unnecessary quotes. – user2864740 Oct 10 '20 at 02:02
  • @user2864740 - I guess, you're absolutely right. I'm now looking how to convert my queries using pdo with parametrized queries. I think I'm on the right path, just not sure how to convert: `$thisDB = new MyDBClass( $dbname['db_name'], $dbuser['db_user'], $dbpwd['db_pass'] );` to use `pdo`. I tried `$ThisDB = new PDO("dblib:host=mssql;dbname=$dbname", "$dbuser","$dbpwd");` and it's not working. Also, not sure if I have to actually add another pdolib.php file for example and include similar functions to my current existing class MyDBClass: it has `odbc_connect(), odbc_result() and such`.. – Korla Oct 10 '20 at 03:37
  • @user2864740 - Is that what I need, right ? https://github.com/drkbcn/PHP-PDO-MsSQL-Class – Korla Oct 10 '20 at 07:18

0 Answers0