[Esapi-user] Fortify vs ESAPI 2.0

Jim Manico jim.manico at owasp.org
Wed Feb 2 23:42:12 EST 2011


I very strongly disagree, Chris. Utility functions like this should have
validation built in (like our file validation APIs). There is no excuse
to NOT add this validation in - especially for public utility functions.

We are a security library after-all...

McGraw was right all this time. Build Security In!

- Jim


> 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