Skip to content

Commit b720cc2

Browse files
committed
[1223] Change FormattedMessage pattern heuristic
We change the order in which `FormattedMessage` checks the format of the provided pattern: we first check for the presence of `{}` placeholders and only then for `java.util.Format` specifiers. This eliminates the need for a potentially exponential regular expression evalutation, which was reported by Spotbugs (#1849). The Javadoc and documentation were improved to clarify the heuristic used by `FormattedMessage`. Closes #1223. Remark: since `FormattedMessage` used the **same** regular expression as `java.util.Format`, if a message uses `java.util.Format` specifiers, it is still vulnerable to a ReDOS.
1 parent 8c92ffe commit b720cc2

File tree

2 files changed

+37
-11
lines changed

2 files changed

+37
-11
lines changed

log4j-api/src/main/java/org/apache/logging/log4j/message/FormattedMessage.java

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.util.Arrays;
2222
import java.util.Locale;
2323
import java.util.Objects;
24-
import java.util.regex.Pattern;
2524

2625
/**
2726
* Handles messages that contain a format String. This converts each message into a {@link MessageFormatMessage},
@@ -30,9 +29,6 @@
3029
*/
3130
public class FormattedMessage implements Message {
3231

33-
private static final String FORMAT_SPECIFIER = "%(\\d+\\$)?([-#+ 0,(\\<]*)?(\\d+)?(\\.\\d+)?([tT])?([a-zA-Z%])";
34-
private static final Pattern MSG_PATTERN = Pattern.compile(FORMAT_SPECIFIER);
35-
3632
private final String messagePattern;
3733
private final Object[] argArray;
3834
private String formattedMessage;
@@ -168,7 +164,27 @@ public String getFormattedMessage() {
168164
return formattedMessage;
169165
}
170166

167+
/**
168+
* Gets the message implementation to which formatting is delegated.
169+
*
170+
* <ul>
171+
* <li>if {@code msgPattern} contains {@link MessageFormat} format specifiers a {@link MessageFormatMessage}
172+
* is returned,</li>
173+
* <li>if {@code msgPattern} contains {@code {}} placeholders a {@link ParameterizedMessage} is returned,</li>
174+
* <li>if {@code msgPattern} contains {@link Format} specifiers a {@link StringFormattedMessage} is returned
175+
* .</li>
176+
* </ul>
177+
* <p>
178+
* Mixing specifiers from multiple types is not supported.
179+
* </p>
180+
*
181+
* @param msgPattern The message pattern.
182+
* @param args The parameters.
183+
* @param aThrowable The throwable
184+
* @return The message that performs formatting.
185+
*/
171186
protected Message getMessage(final String msgPattern, final Object[] args, final Throwable aThrowable) {
187+
// Check for valid `{ ArgumentIndex [, FormatType [, FormatStyle]] }` format specifiers
172188
try {
173189
final MessageFormat format = new MessageFormat(msgPattern);
174190
final Format[] formats = format.getFormats();
@@ -178,14 +194,13 @@ protected Message getMessage(final String msgPattern, final Object[] args, final
178194
} catch (final Exception ignored) {
179195
// Obviously, the message is not a proper pattern for MessageFormat.
180196
}
181-
try {
182-
if (MSG_PATTERN.matcher(msgPattern).find()) {
183-
return new StringFormattedMessage(locale, msgPattern, args);
184-
}
185-
} catch (final Exception ignored) {
186-
// Also not properly formatted.
197+
// Check for non-escaped `{}` format specifiers
198+
// This case also includes patterns without any `java.util.Formatter` specifiers
199+
if (ParameterFormatter.analyzePattern(msgPattern, 1).placeholderCount > 0 || msgPattern.indexOf('%') == -1) {
200+
return new ParameterizedMessage(msgPattern, args, aThrowable);
187201
}
188-
return new ParameterizedMessage(msgPattern, args, aThrowable);
202+
// Interpret as `java.util.Formatter` format
203+
return new StringFormattedMessage(locale, msgPattern, args);
189204
}
190205

191206
/**
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3+
xmlns="http://logging.apache.org/log4j/changelog"
4+
xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.2.xsd"
5+
type="changed">
6+
<issue id="1223" link="https://github.com/apache/logging-log4j2/issues/1223"/>
7+
<description format="asciidoc">
8+
Change the order of evaluation of `FormattedMessage` formatters.
9+
Messages are evaluated using `java.util.Format` only if they don't comply to the `java.text.MessageFormat` or `ParameterizedMessage` format.
10+
</description>
11+
</entry>

0 commit comments

Comments
 (0)