How to avoid SQL Injection attack in this query?

Asked

Viewed 357 times

1

In this case, I am passing WHERE through the $Where variable. Can the system undergo SQL Injection? How to avoid it in this case?

For example: select nome from usuario where $where;

$where = "cod = 10";

public static function getUsuario($where){
    return Usuario::all(array(     
        'select' => 'nome',
        'conditions' => array($where)
    ));
}
  • Is it a framework? Or your code?

  • Using PHP Activerecord + Smarty

  • 1

    So? From foot to hand without seeing any more code? Yes, you are one step away from getting an SQL Injection. However, if any of your frameworks use mysqli_* think that no. By the yes, by the no, I escaped this input just to not have surprises at the end of the day :)

  • What is the advantage of "escaping" the input?

  • Escaping input helps ensure that the data contained in $where are actually given, not code. But I do not know if it is "bulletproof" (and anyway, if these data are already being escaped again will provoke a double escape, which is bad). Where is the value of $where? User’s?

  • From a function that returns the field(name) + value(that comes from an input).

  • By the way, this code there is from Activerecord or Smarty (or both, if one was built on top of the other). Do you have any documentation for us to look up? In fact, if the variable includes a value that came from browser, this value has to be sanitized (both at the entrance - to prevent SQL Injection - and at the exit - to prevent XSS [not applicable in your case]). The framework documentation should help clarify that first part, if you find it post next to the question, we can help to "decipher it" rsrs.

  • Eh a framework made by a development team guy here. It eh a mix of various things rsrs

  • The Where value comes from a Return "name + value", where value is the input value

Show 4 more comments

2 answers

3


Whether it can occur or not I do not know answer, will depend on framework used. But avoid should be much easier, it’s just validate the field, see if its format matches what you expect from it (and therefore will not contain malicious code, or whatever code it is, and yes dice).

Your field $where is composed of the concatenation of $nome and $valor, right? A name is usually composed of letters, numbers and underscore (_). A value is usually composed of digits, maybe a minus on the front and maybe a point. And I’m guessing you concatenate them using a =, right? Then:

  1. Valide the $nome as a name; even though it comes from your own code (it costs nothing, it’s like "sanity test":

    if (preg_match("/^\w+$/", $nome)) {
        ...
    } else {
        /* Não continue! Reporte um erro! */
    }
    
  2. Valide the $valor as being a number; the example below is for a simple decimal, in Brazilian/European (non-American) format, adjust it according to your needs:

    if (preg_match("/^-?\d+(\.\d{3})*(\,\d+)?$/", $valor)) {
        ...
    } else {
        /* Não continue! Reporte um erro! */
    }
    
  3. Concatene $nome and $valor; do not use quotes around the number:

    $condicao = $nome . " = " . $valor
    
  4. Alternatively, if the $valor is a string that can contain - and /, in addition to letters, numbers and underscore (and that’s all):

    if (preg_match("/^(\w|\/|-)+$/", $valor)) {
    

    And to concatenate:

    $condicao = $nome . " = '" . $valor . "'"
    

    In this case, we know that - nor / when within a string are enough to cause problems (a quote or maybe a \ would), and as there is being used a LIKE (case in which _ would be a problem), so it should be safe.

Etc. If $nome has a correct value, $valor has a correct value, the concatenation has the correct syntax, and all the elements of your array $where were mounted in the correct way, so there is no SQL Injection to occur. Whether the components have been "escaped" or not, it makes no difference, as the exhaust will leave them unchanged.

Validating the values is a better alternative than accepting any input format and trying to sanitize it. Even, sanitizing something that you don’t know what it is is reckless (it goes that you "escaped" correctly when you put it in the bank, but when you take it out of it - and "de-capsize" - you included it in the response HTML, without any checks, the result will be an XSS or similar...). Use this strategy whenever the input data format is predictable and well defined, and you only have to worry about the most open fields (free text, for example).

  • my value can be string and integer, and with contain "-" and "/"...

  • @Paulocosta I updated the answer with an example. As far as I know, none of these characters can cause problems, as long as the value is inside the quotes, as if expected from a string (if outside, a -- could cause problems). As I said at the end, this answer is only valid for when the field is closed (i.e. it has a rigid format), if it was opened (anything goes) then it is more complicated. The ideal would be to use parameterized sweethearts, but as we do not know if your framework supports this or not (by the "way" of your code I would say not) this is the most guaranteed means.

  • Very good your answer! Thank you

  • If I want my String to have "." and " _ "??

  • @Paulocosta _ is already contemplated by \w. The . needs to be escaped with \ (it is a special character in regex). The result is then: /^(\w|\/|-|\.)+$/

0

This code only has SQL Injection problems if this parameter comes from user input.

If this Where does not come from the browser / user, no problem.

If you are user input, you should always use escape in php, or else parameters in queries, never direct input.

Browser other questions tagged

You are not signed in. Login or sign up in order to post.