3

Consider the following pair of snippets, both do the same essentially.

<html>
<body>
    <?php
    if(isset($_POST["firstName"]) && isset($_POST["lastName"])){
    //I'm copying the POST variable to a local one.
    $firstName = $_POST["firstName"];
    $lastName = $_POST["lastName"];     
    echo "<h1>Thank you for taking the census!</h1>";
    echo "On behalf of Sergio's Emporium, we name you: " . $firstName . $lastName . ", conquerer of worlds!";
    //Here I'm just pulling it from the POST info.
    echo "I think that's fitting since you're a " . $_POST["item"];
    }
    else {      
    echo "You didn't write in the necesarry information.";      
    }
    ?>
</body> 
</html>

Which is better to use (from a security standpoint) and which one is encouraged to be used by standards.

Since I'm new to PHP this is something that's yanking my chain. Thanks guys! :)

Justin Johnson
  • 30,978
  • 7
  • 65
  • 89
  • Justin is this question really subjective? I thought there was a guideline on best practices for this. –  Feb 13 '10 at 23:42
  • See Pascal MARTIN's answer. It's not how you access it that matters, it's how you use it. Also, guidelines are still largely subjective. – Justin Johnson Feb 13 '10 at 23:45
  • This is very subjective, as each person has a different opinion of what sanitized is, and whether you should do it in the Global variable or local variable. – Tyler Carter Feb 13 '10 at 23:45
  • I think Pascal MARTIN's answer gives you a very clear and concise opinion on the subject. It makes sense. Thanks a million guys. Hopefully other newbies can find this question in the future. –  Feb 14 '10 at 00:00

3 Answers3

7

I would say none of those two solutions change anything from a security point of view, as long as you properly :

  • Filter / validate input
  • and Escape output.

Here, as you are outputting some HTML, it might be useful to escape your data with htmlspecialchars, for instance ;-)


To facilitate that, some people like to consider that :

  • $_POST contains the raw input
  • and some local variable are used to contain the filtered input -- i.e. that you can use "safely" in the rest of your script.
Pascal MARTIN
  • 395,085
  • 80
  • 655
  • 663
  • Aha aha. That 4th bullet point makes a lot of sense. :) Is that how the pros do it? –  Feb 13 '10 at 23:46
  • I sometimes uses this solution *(and it's sometimes enforced by frameworks, in a way, I suppose)* ; not always : depends on the size of my PHP code -- if I'm only writing a quick'n dirty script, I don't do this way, but for a large/full application, it sometimes help ^^ – Pascal MARTIN Feb 13 '10 at 23:49
  • I like to put verified/sanitized/escaped data into arrays, named for the purposes the data has been altered for. Like, for example, `$sqlData`, `$htmlData`, etc... - Makes sure I don't accidentally use insecure data. - And I consider variables in the "global" scope just as unclean as those in the superglobals (`$_POST`, `$_GET`, etc..). Old habit from the days of *register_globals*. – Atli Feb 14 '10 at 01:38
-1

I believe you should because you should do some sort of santizing to the post vars then assign to a local var

JasonDavis
  • 48,204
  • 100
  • 318
  • 537
-1

According to the performance guru's at google, PHP variable copying should be avoided as much as possible: http://code.google.com/speed/articles/optimizing-php.html

Personally, I like it when I can clearly see at the top of the script which variables the script expects from the request so i used to write copies of the $_REQUEST and friends in the top:

<?php
    $req_param1 = $_REQUEST['param1'];
    ...
    if (isset($req_param1)) {
        ...
    }
    ...

Nowadays, I do it differerntly. I typically use define() or in a class, const to define the names of the parameters I expect to get from the request. I can then search for those in the code to see where I actually refeernce them:

define('REQ_PARAM1', 'param1');
...
function foo(){ 
    if (isset($_REQUEST[REQ_PARAM1])){
    ...
    }
    ...
}

example with class:

class MyClass {
    const REQ_PARAM1 = "param1";
    ...
    function foo(){
        if (isset($_REQUEST[MyClass::REQ_PARAM1])){
            ...
        }
    }
}
Roland Bouman
  • 31,125
  • 6
  • 66
  • 67
  • 4
    The "variable copying should be avoided" is not true with recent versions of PHP, if I'm not mistaken : PHP uses copy-on-write, which means the variables will actually really be copied when one "version" is modified. – Pascal MARTIN Feb 13 '10 at 23:43
  • 2
    Correct. There is really not a disadvantage to copying variables. The PHP community has actually criticized Google's paper on that. – Tyler Carter Feb 13 '10 at 23:44
  • Thanks, good to know! I must say, I typically don't worry too much about it too, I simply haven't been in any situation where it mattered. – Roland Bouman Feb 13 '10 at 23:44
  • 2
    As a sidenote, using $_REQUEST is highly discouraged as it can cause undefined behavior and security issues. – stan Feb 13 '10 at 23:46
  • 1
    oh dear, I guess I really blew this one :( well thanks stanislav, I was unaware of those issues but some googling quickly found me some background. Thanks for pointing it out. – Roland Bouman Feb 13 '10 at 23:52
  • Thanks for helping Roland. I won't downvote your answer because it still serves newbies on what not to do. Everyone learns every day. :D –  Feb 13 '10 at 23:57