-4

I love using PDO in PHP but i don't like bind because there is addition of codes etc.. But we cannot ignore the SQL injection and other security holes.

I use this PHP wrapper class PHP PDO Wrapper Class

I heard escaping greatly prevents from SQL injection (is it correct)?

i heard doing HTML special chars completely don't prevent SQL injection?

Can i get the way to escape the data that i get from POST ?

For example i use to insert in database like this using run statement (Using PHP wrapper class)

$firstname=$_POST["first_name"];
and many more variables

global $db;

$db->run(sprintf("INSERT INTO users (UserGroup, UserEmail, UserName, UserToken, UserFirstName, UserLastName, UserPassword, Verified, SignupDate, UserIP) VALUES ('1', '%s', '%s', '%s', '%s', '%s', '%s', 'Y', '%s', '%s')", $email, $username, md5(time()*rand(1, 9999)),$firstname, $lastname, $password, time(),$_SERVER[REMOTE_ADDR]));

Is this above code vulnerable to sql injection and are there any security holes

Elwinar
  • 9,103
  • 32
  • 40
user3689984
  • 5
  • 4
  • No, this is just as vulnerable. Please try to understand [*why*](http://en.wikipedia.org/wiki/Sql_injection#Incorrectly_filtered_escape_characters) string interpolation is vulnerable to SQL injection; then you will understand why your code is also vulnerable. – Oliver Charlesworth Jun 01 '14 at 11:23
  • 1
    You don't like bind? What a strange statement. – The Blue Dog Jun 01 '14 at 11:24
  • 1
    Also, please think about what the relationship is between HTML special chars and SQL injection (hint: there is no relationship). – Oliver Charlesworth Jun 01 '14 at 11:25
  • There is no additional code. One line to bind a parameter or one line to escape a value. It's still one line of code. Only the bind line is better. ;) – GolezTrol Jun 01 '14 at 11:27
  • The Blue Dog actually i don't like prepared statment is there any probelm? – user3689984 Jun 01 '14 at 11:29
  • 1
    @user3689984: So, you don't like bind and you don't like prepared statements, yet you want your code to be secure. Yes, I'd say there's a problem somewhere. – The Blue Dog Jun 01 '14 at 11:34
  • Unfortunately, PHP’s standard extensions are still quite low-level and many developers think they are smart when programing on such a low level. However, there are many libraries/frameworks out there that do a great job in abstracting these low-level action. For example, if you would use [ORM](http://en.wikipedia.org/wiki/Object-relational_mapping), you could just do something like `new User(array('UserEmail' => $email, …))` and everything else is handled by the library/framework. – Gumbo Jun 01 '14 at 11:35
  • @blue dog my question is how to escape it man don't jerk – user3689984 Jun 01 '14 at 11:36
  • @user3689984: If you prepare and bind then you don't need to escape anything, that's the whole point. – The Blue Dog Jun 01 '14 at 11:40
  • I just read that the [PHP PDO Wrapper Class you have mentioned](http://www.imavex.com/php-pdo-wrapper-class/) already supports abstraction for insert. So what’s wrong with `$db->insert("users", array("UserEmail"=>$email, …));`? – Gumbo Jun 01 '14 at 11:42
  • Gumbo yes nothign is wrong with it??i just want to know is that good enough to handle sql injection ..? thanks you read the class – user3689984 Jun 01 '14 at 11:50
  • @user3689984 Yes, it is. That’s the whole purpose of such an abstraction. But note that with this particular class injection is still possible via the table name as they fail to use a prepared statement when querying the table columns (see `filter` function). But it’s safe if the table names are hard-coded. – Gumbo Jun 01 '14 at 12:13
  • +1 gumbo thanks...they are actually hardcoded as user never set....your technique is really awesome ..thanks – user3689984 Jun 01 '14 at 12:59

2 Answers2

1

Your example is still vulnerable to SQL injections as you’re building the statement using user-provided data. In the resulting statement you can’t say which portions were provided by the developer and which by the user.

That’s why people suggest prepared statements and/or parameterized statements where the developer provided portion (SQL statement) and the user provided portion (data parameters) are strictly separated so that they won’t mix.

However, the PHP PDO Wrapper Class you have mentioned does already support this. Here’s an example for your INSERT statement:

$attrs = array(
    "UserGroup"     => 1,
    "UserEmail"     => $email,
    "UserName"      => $username,
    "UserToken"     => md5(time()*rand(1, 9999)),
    "UserFirstName" => $firstname,
    "UserLastName"  => $lastname,
    "UserPassword"  => $password,
    "Verified"      => "Y",
    "SignupDate"    => time(),
    "UserIP"        => $_SERVER['REMOTE_ADDR'],
);
$db->insert("users", $attrs);
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
Gumbo
  • 643,351
  • 109
  • 780
  • 844
0

You are talking here about two kinds of injection :

  1. SQL injection, which use special characters like " or ' to modify the request being executed. It can be avoided by escaping the values in your request, or by preparing your statements (see here for a complete explanation).
  2. HTML injection, which use variables displayed in the HTML to inject code directly into the page. It can be avoided by using htmlspecialchars, and has nothing to do with SQL injection.

You can't use htmlspecialchars to prevent SQL injection. The simpler way to do this would be to prepare your statements (see here). Even if you don't like it, there is not that much code added and it's the most secure way to do it.

You can also use PDO::quote (see doc here). Your request is vulnerable, but you can just use the quote function on each parameter to secure it. I tend to find this much more verbose (compared to preparing and binding), but it you who code.

I read your library page, and didn't found the documentation that helpful, so I read the code and found that is literally extends PDO, so you can use the quote function from PDO with it (or preparing, binding, etc).

Community
  • 1
  • 1
Elwinar
  • 9,103
  • 32
  • 40