During the refactoring of the LogFormatUtils a potential bug has been introduced in 5.3.12 in commit 90fdcf88d832eb6d8f635d3aa727c762b273845b.

The parameter limitLength in the methodformatValue(...) is used when calling the overloaded method, but it is used for the parameter replaceNewlines that has a totally different meaning. This is on the one hand confusing and on the other hand could affect the fix for CVE-2021-22096

Comment From: rstoyanchev

@mum-viadee thanks for you comment. This was an intentional change, adapting the existing method to the new, three argument, formatValue method, with the idea that when limitLength is true, we also default to replacing newlines, both of which aim for more compact output.

In the framework, this is used in codecs for varied output of the request or response body at DEBUG vs TRACE level. Is that where you are seeing the impact?

Comment From: mum-viadee

I was searching for commits clarifying the impact of CVE-2021-22096 on our application. The commit looked like a fix for CR/LF injections. Therfore I was afraid, that this was a bug because it looked like the CR/LF were only removed when limitLength was true on the existing code.

Comment From: senglu

I think that there is other bug (change in behaviour) in the commit.

Version before changeset truncate to size 100 only when limitLength was true: return (limitLength && str.length() > 100 ? str.substring(0, 100) + " (truncated)..." : str);

New version contains:

public static String formatValue(@Nullable Object value, boolean limitLength) {
      return formatValue(value, 100, limitLength);  // 100 goes to maxLength
}
...
        if (maxLength != -1) {
            result = (result.length() > maxLength ? result.substring(0, maxLength) + " (truncated)..." : result);
        }

So it sets maxLength to 100 regardless of limitLength value, so it will truncate result in all cases for two parameters formatValue.

Fix:

public static String formatValue(@Nullable Object value, boolean limitLength) {
      return formatValue(value, limitLength ? 100 : -1, limitLength); 
}

Comment From: rstoyanchev

@senglu you're looking at an intermediate change. Currently (and in 5.3.12) it is this: https://github.com/spring-projects/spring-framework/blob/ff1485fd8d6581edce2ad9bfbe2eb36d2efae8cc/spring-core/src/main/java/org/springframework/core/log/LogFormatUtils.java#L46

Comment From: rstoyanchev

I've updated the Javadoc to make the intent more clear.

@mum-viadee, thanks for feedback. For the future, please use the usual channels for security related concerns or issues. We don't discuss those in public.