8

I am having trouble fixing a Log Forging issue in Fortify. The issue, "writes unvalidated user input to the log", is being raised from both of the logging calls in the getLongFromTimestamp() method.

public long getLongFromTimestamp(final String value) {
    LOGGER.info("getLongFromTimestamp(" + cleanLogString(value) + ")");

    long longVal = 0;
    Date tempDate = null;
    try {            
        tempDate = new SimpleDateFormat(FORMAT_YYYYMMDDHHMMSS, Locale.US).parse(value);
    } catch (ParseException e) {
        LOGGER.warn("Failed to convert to Date: " + cleanLogString(value) + " Exception: " + cleanLogString(e.getMessage()));
        throw new Exception(e);
    }

    if (tempDate != null) {
        longVal = tempDate.getTime();
    }
    return longVal;
}

private cleanLogString(String logString) {
    String clean = logString.replaceAll("[^A-Za-z0-9]", "");

    if(!logString.equals(clean)) {
        clean += " (CLEANED)";
    }

    return clean;
}

The cleanLogString() method has fixed other Log Forging Fortify issues in my project, however it has no effect on the 2 above.

Any help would be appreciated!

Svetlin Zarev
  • 14,713
  • 4
  • 53
  • 82
Brian Redd
  • 414
  • 1
  • 5
  • 12
  • ParseExceptions can contain the value as part of the string returned by `getMessage` so I suspect calling `cleanLogString` on the value returned by `getMessage` will fix one of the problems. The other problem is happening on the `LOGGER.info` call? – Neil Smithline May 29 '15 at 20:04
  • @Neil Smithline thanks for the response, but adding cleanLogString(e.getMessage()) did not solve the issue for the LOGGER.warn() statement. I added this change into the question as to not cause any other confusion. And correct, the other issue is with the LOGGER.info statement. – Brian Redd May 29 '15 at 21:59
  • My next guess would be that Fortify is not recognizing the `cleanLogString` function as something that sanitizes tainted data. I'm not sure why it would recognize it in some places but not others. Do you have a custom rule for it somewhere? – Neil Smithline May 29 '15 at 22:05
  • @NeilSmithline, unfortunately I do not have access to the Fortify rules. My lead has informed me that the rules are still a work in progress. I'll try and dig further into the rules once they become more stable. Thanks for the idea. – Brian Redd Jun 03 '15 at 20:15
  • Fortify has false positives that you can't get rid of without custom rules. Can you just mark these Not an issue and forget about them? – Neil Smithline Jun 03 '15 at 20:33
  • That is an option, however that will be a decision my lead will have to make. – Brian Redd Jun 03 '15 at 20:50

4 Answers4

4

It is possible to use fortify Java annotations to tell Fortify that the data returned from a sanitizing function is now safe.

When looking at my log forging problems I had strings coming in through a web API and thus had the flags XSS and WEB on my strings. I tried to find annotations that would only remove these flags, but couldn't find any way to remove the WEB flag. The only documentation I've found is the Samples/advanced/javaAnnotation directory.

Since my sanitation method does sanitize strings, I choose to remove all flags. This could be a problem though, as it could hide privacy violations.

@FortifyValidate("return")
private String sanitizeString(String taintedString) {
    return doSomethingWithTheString(taintedString);
}
bfpne
  • 41
  • 1
2

Originally when this question was written our team was using log4j v1.2.8, however we noticed that all the log forging issues disappeared after upgrading to log4j v2.6.2.

Once the log4j version is upgraded the Fortify log forging issues should go away. The cleanLogString() method form the question above is also unnecessary. For example:

LOGGER.info("getLongFromTimestamp(" + value + ")");
Brian Redd
  • 414
  • 1
  • 5
  • 12
1

I know I have run into situations where the complexity of my application would stop any malicious input from working as intended; Fortify does not consider this to be secure. I bet you are running into the same thing.

You are stripping any really useful characters out of the log message, but see what happens if you do some encoding on the output prior to writing to the log.

http://www.jtmelton.com/2010/09/21/preventing-log-forging-in-java/

// ensure no CRLF injection into logs for forging records
String clean = message.replace( '\n', '_' ).replace( '\r', '_' );
if ( ESAPI.securityConfiguration().getLogEncodingRequired() ) {
    clean = ESAPI.encoder().encodeForHTML(message);
    if (!message.equals(clean)) {
        clean += " (Encoded)";
    }
}
Dave C
  • 246
  • 1
  • 9
  • Hey, thanks for the answer @DaveC, but unfortunately my group is using Artifactory for our maven dependencies, and the ESAPI jar is not authorized for our use. I'll let you know how it works if we can manage to get access. – Brian Redd Jun 03 '15 at 20:12
-6

Use reflect or try-catch. Its easy to cheat fortify.

roottraveller
  • 7,942
  • 7
  • 60
  • 65