[Esapi-user] Fortify vs ESAPI 2.0

Chris Schmidt chris.schmidt at owasp.org
Wed Feb 2 23:36:42 EST 2011


Herein lies the problem with most static analysis - a complete lack of
context.. This could potentially be an issue if we are ever passing
untrusted data into this method, but this is more of a documentation issue
that anything else I think. This is a standard utility method that *must
never* be called without first validating the parameters. Short of that you
are talking about adding another layer of path validation here and that is
additional configuration as well as potential difficulty for OS specific
validation functions. I think issues like this should be left well enough
alone and the documentation should state that the parameters of this
*utility method* are not validated and as such *must be validated prior to
calling the method* according to the validation requirements of the calling
context. 

> There are also several potential real findings (path manipulation in our
Base64 encoder)
>
> public static boolean decodeFileToFile( String infile, String outfile )
>     {
>         boolean success = false;
>         java.io.InputStream in = null;
>         java.io.OutputStream out = null;
>         try{
>             in  = new Base64.InputStream(
>                       new java.io.BufferedInputStream(
>                       new java.io.FileInputStream( infile ) ),
>                       Base64.DECODE );
>             out = new java.io.BufferedOutputStream( new
> java.io.FileOutputStream( outfile ) );

In other words if I call the method like this:

   String inFile = request.getParameter("inFile");
   String outFile = request.getParameter("outFile");
   if ( Base64.decodeFileToFile(inFile,outFile) {
      // ...
   }

Then, yes - there is a real security issue there. However, if I am calling
it like:

   if ( Base64.decodeFileToFile(
        ARBITRARY_FILE,
        Constants.TMP_DIR + "valid-file-" + System.currentTimeMilliseconds()
) {
        // ...
    }

Then would you still consider that to be vulnerable code? Perhaps if the
first argument wasn't being validated somehow, it could be possible to write
portions of files out to a predictable location on the hard drive - but you
are controlling that location, and you are keeping outside of the proverbial
sandbox (hopefully) so that in order to get to that file, there would have
to be some type of additional vulnerability (LFI) to really exploit the
weakness. You show me the static analysis that can identify that type of
problem and I will buy you an island.. :)

Maybe I am looking at this too critically, but this is just one of the
"potential issues" reported - and really if I was too spend this much time
(10-15 minutes) analyzing each *potential finding* (and it would likely be
more like 30-60 minutes for a good analysis of each) we would not be
releasing GA until 2046..

Don't get me wrong Jim - I think this kind of thing is at least valuable to
know, and I haven't yet had a chance to actually look at the findings
reported by Fortify, however, I think that due to the nature of the code -
static analysis just really can't offer a super viable and accurate analysis
of ESAPI. It is a library at the end of the day, and while there are likely
things in the library code that have separate issues (like the System.out of
the password) the vast majority of findings that are going to be reported
just don't make sense since any method (if used incorrectly) could generally
be exploited to do something wrong, somehow.

Like you said, we aren't perfect and we definitely don't write perfect code
- the best thing we can do with findings like these is document the intended
purpose and hope like hell that developers pay attention to our warnings and
documents.

Wow - I am starting to write e-mails like Kevin; short novellas. :)
 

On 2/2/11 8:34 PM, "Jim Manico" <jim.manico at owasp.org> wrote:


Chris Schmidt
ESAPI Project Manager (http://www.esapi.org)
ESAPI4JS Project Owner (http://bit.ly/9hRTLH)
Blog: http://yet-another-dev.blogspot.com





More information about the Esapi-user mailing list