1

I've been filtering every variables before using PDO. At that time, I usually escape strings and check its length. If the value is integer or any other numeric value, I try to figuring out the value is really desired type and value.

But after using PDO, only thing I do for security is, set the PDO::PARAM_* as binding option like following..

    $stmt = $this->db->prepare("select * from $dbSessionTableName where acntid = ? and end > now()");
    $stmt->bindValue(1, $_SESSION['account_id'], PDO::PARAM_INT);
    $stmt->execute();

Is this really secure?

GatesPlan
  • 465
  • 1
  • 4
  • 17
  • 1
    This is impossible to answer. Yes - it is secure against SQL injection. But your users can still be vulnerable to session based attacks such as replay attacks or hijacking. – max Sep 22 '16 at 08:15
  • 1
    A point of interest, `$_SESSION['account_id']` is a server var and the value should sanitized already before setting it. Especially if this is a value directly from a database and can be trusted, therefor there should be no need to prepare and you could use [query()](http://php.net/manual/en/pdo.query.php) directly. – Xorifelse Sep 22 '16 at 08:28
  • When I was study about database connection, the only reason I found that I had to sanitize my variables is, to preventing injection. In this point of view, then I really trust about security when using PDO. Am I right? ..or maybe I have too much worry about this subject.. – GatesPlan Sep 22 '16 at 08:32
  • @Xorifelse that's just an awful delusion of yours. – Your Common Sense Sep 22 '16 at 08:51
  • @YourCommonSense What do you mean by that? If `account_id` holds the row id of the logged in user (which I presume it does) there would be no need to prepare if you're *only* using that data. People seem to prepare everything or nothing. There is a middle way out ;) – Xorifelse Sep 22 '16 at 09:09
  • @Xorifelse yes, to prepare everything is the only way to go. Otherwise it will do no help at all – Your Common Sense Sep 22 '16 at 09:11
  • @Xorifelse try to look at it this way: for a more or less large project, you just cannot trace the origins or the alleged safety of the every variable. It will be too much a burden for you to decide, whether some particular variable is safe or not. It's much simpler just to process them all the same way. And even if you will consider every variable, one day this practice will spoil you anyway. – Your Common Sense Sep 22 '16 at 09:22
  • @YourCommonSense And here I was just about to comment that you that you and I were at an impasse, saying that I do believe your previous comment holds marred, especially to new programmers but that is not the way for me as I care for performance. So with trusted, I mean with data you know is valid but *if* I was using an external system I'd prepare it for sure. – Xorifelse Sep 22 '16 at 09:29
  • @Xorifelse so you are doing it wrong. It's a pity. Like many other learners, in pursue for the imaginary "performance" you are losing the real security. – Your Common Sense Sep 22 '16 at 09:31
  • @YourCommonSense Well then, insults aside, lets leave it at that shall we? – Xorifelse Sep 22 '16 at 09:52

1 Answers1

2

There are two answers to your question.

  1. Speaking of binding parameters, yes, it is fully secure against SQL injection and do not require any other validation. And even setting the PDO::PARAM_* as a binding option is not necessary. You can simply make your code as follows and it will be secure as well:

    $stmt = $this->db->prepare("select * from table where acntid = ? and end > now()");
$stmt->execute([$_SESSION['account_id']]);

    note that all old-style "sanitization" practices related to sql injection are not only not necessary but rather harmful and should be avoided.

  2. However, speaking of the $dbSessionTableName variable, it is still insecure as it could be with any other approach. And has to be validated.
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • 1
    I am glad you learned something new. You can check my userinfo, there is a link to PDO tutorial I wrote recently. You will find there a lot more to learn . :) Especially how wonderful PDO could be – Your Common Sense Sep 22 '16 at 08:54
  • I'm qurious the reason of why `dbSessionTableName` is insecure. Even if the variable is not passed from client, its still danger? – GatesPlan Sep 22 '16 at 08:55
  • You see, it is not a matter of the data source. It's a matter of the destination. You shouldn't care of the data origin, you should only care of the query. Like you did with prepared statements. It doesn't matter where your data is coming from - it should be always processed the same way, using placeholders (this is why the guy in the comments is awfully wrong). The same thing is with table name. – Your Common Sense Sep 22 '16 at 09:10