0

I have a php script that takes user input and inserts it into a MySQL database. I want to achieve a couple of things with the insert script, a) make sure only acceptable characters get captured and b) prevent SQL injection.

I have a form element like so:

<input class="form-control" id="mrn" name="mrn" type="text" maxlength="17">

I then have in the php script the following:

// Gather the posted data into local variables
$mrn_demographics = mysqli_real_escape_string($db_connect,preg_replace('/[^a-zA-Z0-9]/', '', $_POST['mrn']));

// Form data error handling
if($mrn_demographics == "" ){
echo "The form submission is missing data.";   
} else {
// End form data error handling
$sql = "INSERT INTO nys_demographics (
    mrn
)
VALUES('$mrn_demographics')";
}

Does this achieve the ability to prevent SQL injection and is the use of the mysqli_real_escape_string superfluous since I am also using the regex in order to capture what I want, which is alpha-numeric characters.

MCP_infiltrator
  • 3,961
  • 10
  • 45
  • 82
  • Nope, always `mysqli_real_escape_string()` anyways, it can't hurt. And in actuality, you should make your queries using prepared statements. – Evan Carslake Apr 24 '17 at 22:21
  • 5
    Forget `mysqli_real_escape_string()`, use [prepared statements](https://secure.php.net/manual/en/mysqli.prepare.php) with bound parameters. – Alex Howansky Apr 24 '17 at 22:22
  • @EvanCarslake Are you saying it *needs* to be escaped, or it's a good idea to escape it anyway? – shmosel Apr 24 '17 at 22:23
  • Saying it *needs* to be escaped isn't the right way to word it, a better way to say it would be that it is beyond a great idea to do so. Use prepared statements anyways. – Evan Carslake Apr 24 '17 at 22:25
  • `mysqli_real_escape_string()` is **not** sufficient to prevent SQL injection. Filtering to a known subset of values is good, but using bound parameters is your only real choice. – Alex Howansky Apr 24 '17 at 22:29
  • 1
    Ref. http://stackoverflow.com/a/60496/2864740 (Some implementations may choose to "escape"; regardless, using placeholders is THE way to write 99% of SQL text queries and is proven effective to mitigate first-order attacks. Talking about other methods most of the time - such as now - is a boring tangent to a solved problem.) – user2864740 Apr 24 '17 at 22:31
  • 3
    Also, *validation* should **not** be confused with SQL injection safety. Validation is to ensure a business rule is met. SQL injection mitigation is to ensure that SQL will save the expected data - ie. quotes, or whatever - and not allow SQL injection for *any* input data, regardless of what business rules/filters may previously have (been thought to have) been applied. A correctly written program will do *both* business validation and apply the proper use of SQL - consistently. – user2864740 Apr 24 '17 at 22:34
  • 1
    Regex and escaping data are two different animals and should be inside separate cages. If and when you plan on using this as a register/login system or it's already that, and that you are using a safe hashing method, escaping passwords will do more harm than good; *believe me*. – Funk Forty Niner Apr 24 '17 at 22:53
  • 1
    Since no real answer can come of this and could even be opinion-based, am voting to close the question as too broad. It could also fall as unclear and a possible duplicate of [How can I prevent SQL injection in PHP?](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – Funk Forty Niner Apr 24 '17 at 23:04
  • Possible duplicate of [How can I prevent SQL injection in PHP?](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – Shadow Apr 24 '17 at 23:22
  • I see that there is a question of, is this question duplicative, I have seen the question and answer referenced, I don't think this is a duplicate, since I am, in part after, is `mysqli_real_escape_string()` synonymous to the regex expression, given some think the question too broad, I suppose closing might be necessary but not altogether certain. If the general consensus is to close because of the referenced question and breadth then I'll accept the duplicate closure. – MCP_infiltrator Apr 25 '17 at 00:18

0 Answers0