[nycphp-talk] Promote Secure Coding
Gary Mort
garyamort at gmail.com
Thu May 22 10:26:29 EDT 2014
On 05/21/2014 02:32 PM, Anthony Ferrara wrote:
> First off, I do $name = $_GET['name']. I filter when I know what's
> going to happen with it (such as when it gets bound against a data
> model).
But your not a novice programmer, so this doesn't apply to you. Though
personally, I wouldn't do $name = $_GET['name']. I'd use $name =
filter_input(INPUT_GET, 'name', FILTER_UNSAFE_RAW)
$_GET is fine when you are writing code for a specific platform where
you know in advance what the configuration will be.
However, I'm seeing a lot of linux distro's have started set
filter_default to FILTER_SANITIZE_STRING in their PHP install packages.
I prefer writing code which doesn't depend on specific php.ini
directives in order to work.
>
>
> Example: you're using filter_var with FILTER_SANITIZE_STRING. Do you
> know what that *really* means under the hood?
>
> It's really no better than running strip_tags. And we know that we
> shouldn't be running strip_tags, right?
For this use case, yes we should.
Consider the secretary updating their company website. They have been
told that they need some landing page to say "Welcome <name>" at the top.
The pages are mostly html with a bit of PHP here and there. So they go
to an online tutorial, go through steps 1-4 where they learn about
"hello world" which is a simple little tutorial of
$name = $_GET['name'];
echo "Hello $name";
Well, that's good enough for their needs. A small tweak and now they
have the page and the company sends out e-mail to their clients and all
they have to do is add ?name=XXX on the e-mail.
They ALSO have a horrible cross site scripting issue where someone else
can craft custom links to their website and insert any html and
javascript they want to.
Strip tags is the correct solution for 90% of introductory coding. It
sanitizes the input from a bunch of common exploits. On the downside,
it removes all text from within angle brackets - but 95% of all minor
uses of PHP it means everything just works.
>
> And that's why a solution like you're proposing is bad. It conflates
> filtering with escaping. And that's **always** a bad thing.
Nope, all solutions are based on what is simplest, what performs best,
what is most comprehensive, and what is most appropriate.
My solution maximizes simplest - because that is the most important
thing for beginning programmers. And deals with what is most appropriate
for new programmers.
When people are learning, they put the code up on whatever 'temporary'
web space they have at hand. Whether that is a godaddy site they just
setup, a subdirectory on an active website someone has given them, on
virtual systems running as subdomains for a university
vps123.sandbox.marist.edu [which means that they can set cookie
information for the entire marist.edu domain]
And when their done, they forget about it and leave it running -
publicly accessible and exploitable.
That harms everyone on the internet - because those exploitable bits of
code are used as launching points for denial of service attacks,
controlling e-mail spam botnets, and many more.
Furthermore, unlike most solutions out there, using an anonmous function
tied to a variable means that as programmer learns, they can grow and
make it better.
Eventually, they will get to the point where they learn about including
files and maintaining common code. So then, instead of sticking 4 lines
of code at the top of every php file, they can move it all to a file of
common declarations.
Then they will learn about better ways of filtering data, so instead of
going through every piece of legacy code that they have, they can
instead change it in one place.
$get = function($varName, $filter = 'string') {
$var = filter_input(INPUT_GET, $varName, FILTER_UNSAFE_RAW);
return \Filterus\filter($var, $filter);
}
If instead they started with using a specific filter library and then
discover that the library is no longer supported, then they have to go
throughout ALL their code where they called
$var = \Filterus\filter($var, $filter);
And change every single instance.... of course, this assumes that they
had the patience to wait until they reached the course section on
filtering data.
And for the professional PHP developers, promoting these simple 4 lines
has an added benefit. When the small business grows and reaches the
point of needing to hire a professional - when the professional opens
that code up in their IDE they can easily find every instance of problem
code - since it is all flagged with the \\FIXME: tag which every IDE I
know of automatically will parse and display a list of ever single
instance of those tags[in PHPStorm it's View-->Tools-->ToDo]
I'm certainly open to alternatives that actually address the problem:
novice programmers writing insecure code.
https://github.com/ircmaxell/filterus as it is written right now is not
it. Looking at that page, you have to go all the way to the bottom to
get something a novice programmer might be able to use - and even there
what is written is not of any real use. But then, I assume that you
didn't write it for the novice programmer, so I don't see a problem with
it. Just re-iterating that it doesn't address the same issues my
solution does.
More information about the talk
mailing list