taint View as Markdown

Third, it ignores context. This is related to the first two, but I
think is a separate concern. An example from the taint RFC
(https://wiki.php.net/rfc/taint) is the shell-execution. If the
variable is used in the context of command, one escape function is
needed. If it's used as an argument, another is needed. Detecting
which is not something that's trivial for a language-level taint
function. (…)
Actually, you almost always will want escapeshellarg(). escapeshellcmd()
might be useful for a code like the function example, where you want the
user to explicitely provide several parameters, and you somehow
don't want
to split by spaces and apply escapeshellarg to each. And even then,
there are
non-working edge-cases awaiting to bite you, as shown in the comments.

You have a good point, but escapeshellcmd or "let's change the SQL encoding"
are things that you better avoid, security-wise.

Best regards

While skim reading emails (just got back from holiday), I wanted to add...

All,

On Tue, Sep 15, 2015 at 11:15 AM, Arvids Godjuks
arvids.godjuks@gmail.com wrote:

I fully support your effort to get this into the PHP to be part of core
extensions, or at least one of those that keep up with the language
releases.
This is a very good tool to have, and you can actually run it in production
to catch things that may slipped the stating (things happen). And it's
invaluable during during testing.

I'm against any form of tainting for a few reasons.

First, it's only really useful as a first-order heuristic. A failure
does not indicate a security vulnerability, and a non-failure does not
indicate the absence of one. An example of a false-positive would be
if (preg_match('(^[a-z0-9]+$)', $input)) echo $input;. Taint-based
systems would flag that as a possible XSS, where it's clearly not in
any context.

Personally I would argue that if the echo $input was for printing a variable in a HTML construct, so it MUST go though one of the HTML encoding functions, because that regexp could be changed later.

e.g. an inexperienced developer being told that usernames on a website can now support angle brackets, so "just edit the regexp for the username validation".

This is keeping in mind that the regexp is now usually kept separate from the view code (e.g. MVC style)

An example of a false-negative would be the following:

<a href=<?php echo htmlspecialchars($input); ?>>Bar</a>. If the
input is "foo onclick=dosomethingbad();", an XSS is still executed.

True, but without htmlspecialchars, it will ALWAYS be bad... so while a tainting system won't be able to pick up all problems, it can at least identify more mistakes than it not existing (percentage wise, well that will depend on the programmer).

Maybe you could think of it like having a second pair of eyes looking over your shoulder, politely asking you if you forgot something (a bit like how pair programming helps improve code quality).

Another example of a false negative would be:

$query = "SELECT * FROM foo WHERE bar = " .
mysqli_real_escape_string($c, $input) . ";";

A false positive is potentially OK. A false negative is not.

All of the examples have been about using pg_escape_identifier(), because that adds the quote marks... mysqli_real_escape_string by itself would not be enough to say that it safe for an SQL string :-)

Second, it will encourage the improper pattern of "sanitize" functions
to de-taint input. These are functions which call
mysqli_real_escape_string(htmlspecialchars(whatever(etc($input)))).
No matter what the context the variables are used in. This doesn't
improve security (and in many cases can demonstrably harm it). It just
encourages people to work around it. And that ignores that there's an
untaint() function that they can call on any variable to "silence
the errors".

I don't think a sanitize() function would be implemented though, as it would make a mess of variables being passed though more than once... resulting in things like double html encoding (e.g. &amp;), and these stand out much better than the current sanitize() functions which do things like silently stripping "bad" characters like double quotes and less than signs.

If anything, a proper tainting system would help us move away from these horrible functions (says he having had to remove a couple before now).

Third, it ignores context. This is related to the first two, but I
think is a separate concern. An example from the taint RFC
(https://wiki.php.net/rfc/taint) is the shell-execution. If the
variable is used in the context of command, one escape function is
needed. If it's used as an argument, another is needed. Detecting
which is not something that's trivial for a language-level taint
function. And calling the wrong function defeats the purpose. So if we
can't detect the context, we can't reliably say whether it's safe or
not.

Not sure I understand, if anything this gives the strings a context?

Rather than every variable being a simple string (number, etc), they start off being plain text (or a string defined as a constant in PHP), and their escaping is checked in the context of the function that was called (e.g. exec needing escapeshellarg).

In the hands of an experienced engineer/security researcher, taint
systems can be a really handy tool to help find possible issues. In
the hands of a junior engineer, it can actually make things worse
because it encourages them to just "silence the errors".

While I originally hoped that it would be enabled by default, I think for now it would have to be off by default, and probably only used by "experienced engineer/security researchers".

At the moment, static analysis seems to be the closest alternative I can find (and they are pretty awful).

Considering that it's not a reliable tool, and is only a heuristic, I
would suggest that this be implemented as an extension, and not in the
core of a programming language.

Maybe, but I think with some discussions, having it part of the core language would help get all the other extensions to use it (if needed), and help improve viability.

See it like having Notices present, but off by default (which they are).

The other positive is that distros such as Debian / RedHat will then ship with it by default (disabled), rather than having to ask/hope that they will provide a version with it (keeping in mind that they provide security updates without needing to re-compile).

With that said, if there are engine hooks necessary to implement this
feature better as an extension, I'm all for adding those hooks. I'm
just against adding something to the language that won't actually be
able to accomplish what it promises.

Escaping is always context-sensitive, in every case. Tainting as has
been proposed in the past doesn't (and literally can't in the general
case) know the context. Therefore it can't reliably know the correct
way to escape.

I think it can, but I agree that there are some edge cases that it won't be able to catch, such as:

$url = $_GET['u']; // ?u=javascript:alert(document.cookies);

echo '<a href="' . htmlentities($url) .'">Cookies</a>';

Consider the case of addslashes() and magic_quotes_gpc. The reason
that they are insecure is that they don't know the context that the
variable is used in. This leads to character set issues, as well as
cases where backslashes enter contexts they don't escape (like HTML
output). We learned years ago that context-insensitive escaping
doesn't work and is literally bad. Let's not make the same mistakes
again (even if it's for the correct reasons this time).

I am repeating myself now, but yes, magic_quotes_gpc did not know about the context those variables were going into... tainting on the other hand is checking the variables, which are going into a known context, have been escaped (with a couple of possible issues, but they can be addressed though other methods).

Craig

Good morning,

==8<--------------------------
Taint is blacklisting.

Last time I checked marking all user input as tainted and requiring
"untainting" before usage in sensitive functions is whitelisting and not
blacklisting.

Regards,
Stefan

--
SektionEins GmbH stefan.esser@sektioneins.de
Breite Str. 159 Tel: 0221 / 29282931
50667 Köln Fax: 0221 / 29282935
http://SektionEins.de/

Firmensitz Breite Str. 159 50667 Köln
Registergericht Amtsgericht Köln HRB 59950
Geschäftsführer: Stefan Esser, Benjamin Fuhrmannek, Christian Horchert

Just to add to the white/black listing argument...

I would say that tainting is a whitelist approach, as everything is blocked by default (seen as untainted), and you need to escape your variables depending on the context they will be used in (or go out of your way to say it has already been escaped, for the rare edge cases mentioned in previous discussions).

As a side note, preg_match() is my current favorite blacklist indicator of "code with probable security vulnerabilities."

I completely disagree, preg_match can work in both ways, it can do white or black listing, as Anthony demonstrated:

if (preg_match('/^[a-z0-9]+$/', $input)) {
}

That white lists only certain characters, anything else won't match.

Craig

Hi all,

Am 15.09.2015 um 17:09 schrieb Craig Francis:

2015-09-14 4:44 GMT+02:00 Christopher Owen christopher.owen@live.com:

Please consider making ‘taint’ a first-class feature/extension in PHP 7.0.

I would echo Kalle's suggestion of 7.1.

But I think you will find it hard to get support... I was pushing this a few weeks ago (either the one from Wietse Venema, the one from Matt Tait, or even my own suggestion), but it seems the developers are more interested in features that make them seem cleaver, rather than pointing out their mistakes...

the problem with taint support is to get it 100% right. If you leave one
edge case open, who is to blame? PHP or the developer that was totally
confident the taint support might warn him?

You can grab the following four paragraphs and add it to whatever documentation on taint you might use.

==8<--------------------------
Taint is blacklisting.

Blacklisting, in and of itself and regardless of the form it takes, is an immediate indicator of an application that is prone to security vulnerabilities and/or breakage that can lead to a vulnerability.

With extraordinarily rare exceptions, blacklists are never 100% correct. Even if it can manage to reach 100% accuracy today, a blacklist will be out of date tomorrow due to advances in the field.

While writing software being paired with a blacklist such as taint, performing an appropriate security audit of the software is the only truly effective approach to securing that software.
====8<------------------------

Problem solved. Also, those are ASCII scissors (in case anyone is wondering).

As a side note, preg_match() is my current favorite blacklist indicator of "code with probable security vulnerabilities." If taint is added to PHP without a suitable caveat lector, it merely adds another tool to my psychological profiling arsenal.

--
Thomas Hruska
CubicleSoft President

I've got great, time saving software that you will find useful.

http://cubiclesoft.com/