codeql icon indicating copy to clipboard operation
codeql copied to clipboard

False positive - Log entries created from user input (cs/log-forging)

Open serhatataman opened this issue 11 months ago • 1 comments

Description of the false positive

The user input/log string is handled inside an extension log method and sanitized. We are getting a false positive warning from our Logging extension method. The warning message is: Log entries created from user input (cs/log-forging)

Code samples or links to source code

This is how the logger is called and the place where it throws this log warning message. (However, note that we do our message sanitization inside our extension method)

_logger.Feil($"Fant ingen for oppgitt år {aarstallStreng}", nameof(MetodeArrsbonus), correlationId); . Note that Feil is our extension method and it looks like this:

public static void Feil(this ILogger logger, string message, string metodeNavn, string correlationId)
    {
        logger.LogError("CorrelationId: {CorrelationId}, MetodeNavn: {MetodeNavn}, Melding: {Melding}, Tidspunkt: {Tidspunkt}", correlationId, metodeNavn, RemoveInvalidCharacters(message), DateTimeOffset.Now);
    }

And this is our sanitization method:

private static string RemoveInvalidCharacters(this string message)
    {
        message = message.Replace(Environment.NewLine, "");

        var invalidChars = new HashSet<char>("&^$#@+;<>*");
        StringBuilder washedMessage = new StringBuilder(message.Length);
        foreach (char x in message.Where(c => !invalidChars.Contains(c)))
        {
            washedMessage.Append(x);
        }
        return washedMessage.ToString();
    }

This is the error that occurs: image

What did I expect to happen? Since we sanitize the input in our extension method, the error should not appear.

How can you reproduce it? override logging method, pass inn variables to that override method as string and do string sanitization inside that override method.

serhatataman avatar Mar 06 '24 12:03 serhatataman

Hi @serhatataman 👋

Thank you for reporting this to us. That does indeed look like a false positive and we will track this issue internally. However, I cannot give you any estimate for when a fix will be available.

mbg avatar Mar 06 '24 13:03 mbg