[Esapi-user] Fortify vs ESAPI 2.0

Kevin W. Wall kevin.w.wall at gmail.com
Thu Feb 3 10:58:04 EST 2011


On 02/03/2011 07:56 AM, Jim Manico wrote:
> Taint tracking is incredibly dangerous to depend on. I feel that secure
> coders should treat configuration data, database data, request input,
> and most any other system data as tainted. Even post-validated data should
> be treated as "tainted" if you want your code to stand the test of time.

It is not the taint *tracking* that is incredibly dangerous, but rather
the nonchalant way developers go about untainting. If seen the equivalent
of something like this all too often (pardon my perl; am a bit rusty):

    #!/usr/bin/perl -T
    use strict;
    use CGI;
    use CGI::Carp qw(fatalsToBrowser);

    my $cgi    = CGI->new();
    my $comment = $cgi->param('usercomment'); # $comment now tainted

    if ($comment =~ /^(.{1,80})$/) {
       $comment = $1;   # comment is now untainted
    } else {
       croak "Comment too long; max length is 80 characters";
    }

which is completely worthless as all it does is check the *length* of
input. (Developers say "It's a comment; they can use whatever they
want." Sigh.)

No, the problem is not taint *tracking*, it's the developer inattention
to the details of properly untainting tainted variables. But without
taint checking, they wouldn't even try.

Having stated this, I agree that taint checking is not a substitute
for good, secure coding practice. But frankly, given the inability
of the masses (or perhaps more likely, the pressure to get it out
the door by Friday's ship date), they tend to get sloppy and/or
take shortcuts. So I definitely *do* want taint checking there to
watch their back. And ESAPI as well.

I only wished that other programming languages offered taint checking.
(BTW, taint checking is the basic mechanism that Fortify and I imagine
other static code analysis tools use when there are doing their
"source to sink" analysis, but you [Jim] probably already realized that.)

> If ESAPI offers a public function, we should treat those inputs as
> tainted. Again, even post validated data. Otherwise it does not belong
> in our "production quality security library".

In general use, but not *always* because sometimes it doesn't matter, or
you just don't want that.

For example, it irks me when I find that web sites don't allow certain,
supposedly "harmful" characters in a user's passwords. I don't know how
many times I've seen this; they exclude characters such as quotes, <, >,
=, %, etc. I cringe when I see that; it's definitely one of my hot
buttons. There is absolutely NO REASON to treat passwords that way as
it reduces the possible password space. (Even worse is when they
restrict it to some maximum length.)

To start with, usually that's a dead give-away that they are encrypting,
rather than hashing their passwords (or worse, and just storing them as
plaintext somewhere). Worse, it's usually an indication that at some point,
they are probably exposing that password to someone (e.g., help desk personnel)
so they can confirm it if a user forgets their password. [And don't EVEN
get me started on password "security questions".] Instead they should allow
any and all input and immediately hash it with a secure hash and then
compare the (possibly base64-encoded) hash of the input password with
the stored hash.

I've not looked, but if I find ESAPI code disallowing certain characters
in passwords, you all will hear a very loud scream from Columbus, Ohio,
and it won't be "Go Bucks!".

Likewise, when you are encrypting or decrypting data, you have to
allow *anything*. (ESAPI: "Er, sorry sir; I'm afraid I can't allow you to
encrypt that potentially dangerous payload as we have detected characters
that may cause XSS or lead to SQLi."; Developer: "FU. It's binary data
that I'm encrypting you moron.")  OK, you get my point. But I think that
Chris was trying to make a similar point and he just wanted the documentation
to be clear as to the dangers therein.

I think a better approach might be when ESAPI offers a public function
that does NOT validate data, we should just name it in some specific
way, for example, by giving each such method (or class) an 'unsafe'
prefix. So in the case of the Base64 class, we could rename the unsafe
method to 'unsafeDecodeFileToFile' and then it becomes a bit more obvious
as to what you are getting. This would benefit those who are not prone
to RTFM. (Of course, in this particular case, there's still the issue
of backward compatibility, so deprecation would be the proper approach
here.) Of course, we don't have coding standards like that in ESAPI,
but we probably should. (Ditto with code that is not thread-safe.)

-kevin
--
Kevin W. Wall
"The most likely way for the world to be destroyed, most experts agree,
is by accident. That's where we come in; we're computer professionals.
We cause accidents."        -- Nathaniel Borenstein, co-creator of MIME


More information about the Esapi-user mailing list