NYCPHP Meetup

NYPHP.org

[nycphp-talk] Promote Secure Coding

Gary Mort garyamort at gmail.com
Wed May 21 13:13:08 EDT 2014


On 05/21/2014 11:44 AM, Pierpaolo D'Aimmo wrote:
> Interesting, thank you for the contribution.
> Same rules can be applied to $_REQUEST and $_POST, but I guess you 
> think that's already clear from what you write in the last comments. 
> Unfortunately, many people I think just want ready-made functions to 
> copy and paste.

I want to create those as seperate articles and then crosslink them 
all.  Yes, the idea is purely for people who are going to cut and paste 
code.  IE all of this would be much better to declare once somewhere 
rather then in every single file - but that all requires at least 3 or 4 
lessons in programming PHP before you get to that point.

$_REQUEST is special in that there is no input stream for it,
http://us1.php.net/manual/en/function.filter-input.php

So a $_REQUEST filter either has to reimplement the GPC logic OR it can 
be done by calling filter_var($_REQUEST[$varName], $filter) - both of 
which are not ideal. I know that I at least like to pretend I think I 
know what code I'm cutting and pasting into my programs does so the 
whole GPC testing logic will look confusing.  Wheras using the $_REQUEST 
variable means you can't unset it for safety.

What I really like about using closures is that it becomes novice 
friendly.  If instead I had written:
function get($varName = '')
{
    return filter_input(INPUT_GET, $varName, FILTER_SANITIZE_STRING);
}


Functionally it performs the same, but once the junior programmer starts 
using multiple files, it will break if it is used in multiple files.

As an experienced programmer,
$myvar = $_GET['myvar'];
and
$myvar = filter_input(INPUT_GET, 'myvar', FILTER_SANITIZE_STRING);

Those 2 lines read the same to me.  The second line reads as "I am 
explicitly filtering the data for my needs" while the first line reads 
as "This is just an example and don't use this in real code".

But beginners aren't going to see that.   And it really frustrates me 
because PHP is such a rich language that it can be used by people who 
have taken a short course on html as well as programmers.  So I feel 
that we[the PHP community] are at fault for insecure PHP programs 
written in the past 10 years.  By not providing and promoting a simple 
means for novices to write more secure code - we allow it to prolifigate.

The secretary updating the company website for a small business is not 
going to bother to go past lesson 3 or 4 when she is told to add an 
input field for subject on the contact form.   She will go as far as she 
needs to figure out she can add $subject = $_GET['subject'] and then be 
done.


> You can make it more complete or be more clear in the "FIXME" line. 
> Also, at least comments shouldn't be self-explained when not talking 
> about them.
> Something like:
> //FIXME: This code is just an example, it's not complete, don't use 
> it, just learn what it does and implement something that suit your 
> real needs.
> // You may want to apply it to other variables as well, or even not 
> use it at all (in some special cases.)
>

Thanks, I'll take a look.  I also have a couple other versions of the 
same code which I want to post with this one.  The most complex is 
around 30 lines to deal with posted variables.  One of the nice things 
with using INPUT_GET was that even if PHP is configured not to create 
the $_GET superglobal, you can always get the original query info from 
it.   Wheras if you configure PHP not to create the $_POST variable then 
you have to use the php://input stream  - so it takes a few more lines 
for that as well as adding a set and exists option to filtering.

My thinking is that it can all be crosslinked.  Promote the simple 4 
liners as the minimum, while providing links to more complex bits of 
code like this one which they can grow into:


     // 30ish lines to sanitize and modify query string variables
     // FIXME: make sure to use custom filters for your needs
     unset($_POST); // Force yourself not to use the global variable
     $post = function($varName, $filter=FILTER_SANITIZE_STRING, 
$action='get',$value=null) {
         // Internal variable to allow updating of input data
         static $pRaw = false;
         // Load the post data from input
         if (!is_array($pRaw))
         {
             $postInput = file_get_contents('php://input');
             echo 'post input is ' . $postInput;
             if ($postInput)
             {
                 parse_str($postInput, $pRaw);
             } else
             {
                 $pRaw = array();
             }
         }
         $return = null;
         if ($action == 'get' & isset($pRaw[$varName]))
         {
             $return  = filter_var($pRaw[$varName], $filter);
         }
         if($action == 'set')
         {
             if (isset($pRaw[$varName]) )
             {
                 $return = $pRaw[$varName];
             }
             $pRaw[$varName] = $value;
         }
         if ($action =='has')
         {
               $return = isset($gRaw[$varName]);
         }
         return $return;
     }


More information about the talk mailing list