Is it safe to use this method?
Let's say I have a file called query.sql
with the following content in it:
SELECT * FROM `users` WHERE `id`!=".$q->Num($_POST['id'])."
And in my php script that has an html form with an input named "id" in it, I do the following trick:
$sql=file_get_contents('query.sql');
$query= eval("return \"$sql\";");
//here follows something like $mysqli->query($query); and so on..
I am not sure about sql injection as I am using prepared statements and $q->Num
doing validation is_int
. But is it safe to use eval this way?
As I understand it, we actually look at "$ {_ POST ['id']}" here, and it calculates some string value entered by the user. And it only becomes dangerous if I pass this line a second time. Whereas I eval string only after user input is just a string and cannot be interpreted as php code by the compiler and no PHP injection is possible.
UPDATE Thank you for suggesting different methodologies and highlighting the need to use prepared statements. But that's not my question. My question is about php injection. Is this use of eval bad? If so, why?
source to share
The eval statement - although I would try to find a way without using eval - is not vulnerable to PHP injection because it is $sql
enclosed in double quotes "
; There is no way to end this quoting with a prepared variable in PHP.
I am not good at sql injection since I am using prepared statements
Isn't it?;) You!
Why are you adding $ id to the request with '.' operator (string manipulation)? If you are really taking advantage of prepared statements, I would expect something likebindParam()
A note on how prepared statements prevent SQL injection: SQL query syntax is kept separate from arguments. So the server will
- parse the SQL query
- apply arguments
Since the request has already been parsed before the arguments are applied, the syntax of the request cannot be processed by arguments.
If you prepared a MySQL query that was created using '.' and external inputs that are potentially vulnerable to SQL injection
What you are doing violates the principles of prepared statements
source to share
No need to use eval
- put in token and replace it:
// file query.sql
SELECT * FROM `users` WHERE `id`!="{id}";
//php
$sql = file_get_contents('query.sql');
$query = str_replace("{id}", $_POST['id'], $sql);
Update
No, it is not secure. Someone can edit your query.sql
script to do anything. You can say "application is internal only" or "I have permissions" or whatever, but at the end of the day there are no guarantees.
source to share
Link to this answer.
fooobar.com/questions/1037 / ...
The main problems with eval ():
Potential unsafe input. Passing an unsafe parameter is a way to fail. It is often not a trivial task to make sure that a parameter (or part of it) is fully trusted.
Trickyness. Using eval () makes the code smart, so it's harder to follow. To quote Brian Kernighan "Debugging is twice as hard as writing the code in the first place. So if you write as much code as possible, you are by definition not smart enough to debug it"
source to share
Consider the following code:
$sql=file_get_contents('query.sql');
$query= eval("return \"$sql\";");
Dangerous points in this case:
-
if the file is
query.sql
changed from what is expected, then it can be used to execute any arbitrary code in your program. -
the file name is hardcoded; if not, an attacker could find a way to download an unexpected file (possibly one from a remote site), again leading to arbitrary code execution.
-
the only reason to use a file to do this (rather than hard-coding the SQL directly into the program) would be because you want to use it as a config file. The problem here is that the syntax in this file is not valid for both SQL and PHP. Because of the way it works in it
eval()
, it also requires the syntax to be exactly correct. Use the wrong quotes or miss one and it explodes. This will most likely result in brittle code that will fail rather than gracefully when the configuration is marginally wrong.
There doesn't seem to be a direct SQL injection attack here, but that's actually the least of your worries when it comes to eval()
.
I have personally worked on projects where code existed that worked very exactly as you described. As a result, there were some nasty bugs in the system that were difficult to fix without bulk rewriting. I would highly recommend dropping this idea and using a sane templating engine as recommended by others in the comments.
source to share