False positive

[silent-sour]

Description of the false positive

C# CWE-117 is incorrectly applied to user input sanitized with {string}.ReplaceLineEndings() instead of {string}.Replace(Environment.NewLine, string.Empty)

**Code sample

var username = authInfo.Username.ReplaceLineEndings();
_logger.LogError("Invalid login attempt: {username}", username);

[silent-sour]

Bump for review

[michaelnebel]

I will take a look.

[michaelnebel]

I think the example might need to be changed a bit as the parameterless overload of ReplaceLineEndings replaces newlines with newlines (using the current environment). That is, we should probably only consider a string to be sanitized using the ReplaceLineEndings method in case the new lines are specifically replaced with something (eg. string.Empty)

var username = authInfo.Username.ReplaceLineEndings(string.Empty);
_logger.LogError("Invalid login attempt: {username}", username);

[michaelnebel]

The issue is fixed here: #17815

[silent-sour]

Thank you for working towards a resolution here.

You are incorrect in your assertion there. The documentation in the source shows the following:

    /// This method searches for all newline sequences within the string and canonicalizes them to the
    /// newline sequence provided by <paramref name="replacementText"/>. If <paramref name="replacementText"/>
    /// is <see cref="Empty"/>, all newline sequences within the string will be removed.
    ///

As such this method takes line endings (CR, LF, CR/LF) canonicalizes them to the local platform line ending, and replaces them with either the replacementText or the empty default value.

[michaelnebel]

Thank you for the quick response.

The snippet of documentation you are referring to is from ReplaceLineEndings(string replacementText) and it states that if you provide string.Empty as an argument to ReplaceLineEndings(string replacementText) then all newline sequences will be removed.

The documentation for ReplaceLineEndings() can be seen here and it says

        /// <summary>
        /// Replaces all newline sequences in the current string with <see cref="Environment.NewLine"/>.
        /// </summary>
        /// <returns>
        /// A string whose contents match the current string, but with all newline sequences replaced
        /// with <see cref="Environment.NewLine"/>.
        /// </returns>

which does't remove newlines.

[silent-sour]

Appreciate it, as I guess I didn't see that overrides' documentation.

[michaelnebel]

The improvement has been merged.