2

Ive been informed this is not safe to use in regards to sql injection:

preg_replace("/[\"'%()@$.!&?_: #\/-]/","", mysql_real_escape_string($_GET['var']));

what would be the difference in the following change?

mysql_real_escape_string(preg_replace("/[\"'%()@$.!&?_: #\/-]/","", $_GET['var']));

how about if no database connection was made, for example exec was called, how could this be bypassed? working example? a better way to guard against injection?

$theVar = preg_replace("/[\"'%()@$.!&?_: #\/-]/","", mysql_real_escape_string($_GET['var']));
exec("zip -r archived.zip ./".$theVar."/");
quick_learner42
  • 123
  • 1
  • 4

3 Answers3

1

The only sure way to guard against SQL injection is by using parameterized queries. See: SQL Injection Prevention Cheat Sheet by OWASP.

1

It all depends on the context in which the user supplied data is inserted into.

In case it’s not a SQL string literal, mysql_real_escape_string would be useless as there is no string the attacker has to break out from and something like a UNION SELECT … with certain whitespace characters instead of the filtered space would still be possible.

Exploiting the exec example is even easier:

var=;+evil+command;+cd+

This would result in:

zip -r archived.zip ./; evil command; cd /

Here escapeshellarg would be the proper escaping function:

exec("zip -r archived.zip ".escapeshellarg("./".$theVar."/"));

However, you would still have to validate $_GET['var'] even though you’re using escapeshellarg to make sure the entered value is what you’re expecting, e.g., a valid file system path.

I hope you see that there is no universal escaping or filtering function. You always have to take the context into account, in which the user supplied data will be inserted into. Each context may has its own rules, special characters, and escape sequences that have to be dealt with separately. You will even encounter multiple levels of contexts that all have to be handled separately and in the right order.

Community
  • 1
Gumbo
  • 2,003
  • 1
  • 13
  • 17
1

This is not SQL injection at all; you are not passing $theVar to a SQL server, but to a shell command. Therefore, mysql_real_escape_string is useless.

You need to employ escapeshellarg, which applies the same concept to a shell string.

In this example,

$theVar = preg_replace("/[\"'%()@$.!&?_: #\/-]/","",  mysql_real_escape_string($_GET['var']));
exec("zip -r archived.zip ./".$theVar."/");

you are not escaping the redirection characters < and >. So (for example) it is possible to overwrite any file that the process executing zip has access to.

Even worse, you are not escaping the command separator character ; . So it is possible to add extra commands (as far as they don't use any of the forbidden characters) that will also be executed.

A better approach would be to see what characters are permissible in a file name, and only accept those, rejecting any string containing forbidden characters. If you allow wildcards (looks like you do), expand those yourself with glob() to obtain a list of files, and check those files for permissions and location by translating each path to a unique canonical pathname using realpath().

Better still, do not allow wildcards. At that point it would also be easy to verify whether the requested file does indeed exist. Shell commands would mostly turn out to be invalid or nonexistent as file names, and be automatically rejected at no additional cost.

LSerni
  • 22,521
  • 4
  • 51
  • 60
  • just because you don't see sql code doesn't mean it isn't or can't be used for sql insert/etc.. – quick_learner42 May 13 '13 at 15:51
  • I beg your pardon, but the issue is not so much in my not seeing SQL code, but in the escaped string being used inside a *shell command*. If you held a Colt to your head and asked "*How do 3D wireless mice work?*", I would *not* feel comfortable in answering you "Just point and click". – LSerni May 13 '13 at 20:02
  • poor reading comprehension on your part I suppose. Or you have not taken the time to read the post at all and just shot from the hip? – quick_learner42 May 14 '13 at 03:11
  • We're not going to agree on this, and indeed, English is not my native language. I have clearly misunderstood for a key point what you only intended as an aside example. So: should I retract my answer (I see little point in editing it adding '*This does not answer your question, but...*', and the `escapeshellarg` suggestion was already given - even approved - anyway), do you wish to downvote it as not useful, or is there something else that's done in these cases? – LSerni May 14 '13 at 05:45
  • I apologize. Good answer – quick_learner42 Jun 03 '20 at 00:40