[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