diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/Rfc5424LayoutTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/Rfc5424LayoutTest.java index 0eaac7c9f8a..76dbe472a90 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/Rfc5424LayoutTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/Rfc5424LayoutTest.java @@ -973,13 +973,67 @@ void testParamNameSanitization(final String paramName, final String expectedStru assertThat(actual).isEqualTo(expected); } + private static LogEvent createLogEventWithStructuredData(final String id, final String type) { + final MutableInstant instant = new MutableInstant(); + instant.initFromEpochMilli(1L, 0); + + return Log4jLogEvent.newBuilder() + .setInstant(instant) + .setMessage(new StructuredDataMessage(id, "MSG", type)) + .build(); + } + + private static Stream testSdIdSanitization() { + return Stream.of( + Arguments.of("validId", "[validId@32473]"), + Arguments.of("event id", "[event?id@32473]"), + Arguments.of("event=id", "[event?id@32473]"), + Arguments.of("event]id", "[event?id@32473]"), + Arguments.of("event\"id", "[event?id@32473]"), + Arguments.of("a] [forged x=\"y", "[a??[forged?x??y@32473]")); + } + + @ParameterizedTest + @MethodSource + void testSdIdSanitization(final String id, final String expectedStructuredData) { + final Rfc5424Layout layout = Rfc5424Layout.newBuilder().build(); + + final String actual = layout.toSerializable(createLogEventWithStructuredData(id, "Audit")); + + assertThat(actual).isEqualTo(formatExpectedMessage(layout, "Audit", expectedStructuredData)); + } + + private static Stream testMessageIdSanitization() { + return Stream.of( + Arguments.of("Audit", "Audit"), + Arguments.of("Au dit", "Au?dit"), + Arguments.of("Audit\r", "Audit?"), + Arguments.of("Audit\n<13>1 - - - - -", "Audit?<13>1?-?-?-?-?-")); + } + + @ParameterizedTest + @MethodSource + void testMessageIdSanitization(final String type, final String expectedMessageId) { + final Rfc5424Layout layout = Rfc5424Layout.newBuilder().build(); + + final String actual = layout.toSerializable(createLogEventWithStructuredData("Audit", type)); + + assertThat(actual).isEqualTo(formatExpectedMessage(layout, expectedMessageId, "[Audit@32473]")); + } + private static String formatExpectedMessage(final Rfc5424Layout layout, final String expectedStructuredData) { + return formatExpectedMessage(layout, "-", expectedStructuredData); + } + + private static String formatExpectedMessage( + final Rfc5424Layout layout, final String expectedMessageId, final String expectedStructuredData) { final String timestamp = DateTimeFormatter.ISO_OFFSET_DATE_TIME .withZone(ZoneId.systemDefault()) .format(Instant.ofEpochMilli(1L)); return String.format( - "<128>1 %s %s - %s - %s MSG", timestamp, layout.getLocalHostName(), PROCESSID, expectedStructuredData); + "<128>1 %s %s - %s %s %s MSG", + timestamp, layout.getLocalHostName(), PROCESSID, expectedMessageId, expectedStructuredData); } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java index 461c707a0af..1cd29906d4d 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java @@ -99,6 +99,11 @@ public final class Rfc5424Layout extends AbstractStringLayout { private static final int SD_PARAM_NAME_MAX_LENGTH = 32; + /** + * Characters that RFC 5424 excludes from an {@code SD-NAME}. + */ + private static final String SD_NAME_EXCLUDED_CHARACTERS = "=]\""; + private static final String LF = "\n"; private static final int TWO_DIGITS = 10; private static final int THREE_DIGITS = 100; @@ -346,9 +351,9 @@ private void appendMessageId(final StringBuilder buffer, final Message message) final boolean isStructured = message instanceof StructuredDataMessage; final String type = isStructured ? ((StructuredDataMessage) message).getType() : null; if (type != null) { - buffer.append(type); + buffer.append(sanitizeMsgId(type)); } else if (messageId != null) { - buffer.append(messageId); + buffer.append(sanitizeMsgId(messageId)); } else { buffer.append('-'); } @@ -558,7 +563,7 @@ private void formatStructuredElement( } sb.append('['); - sb.append(id); + sb.append(sanitizeSdId(id)); if (!mdcSdId.toString().equals(id)) { appendMap(data.getPrefix(), data.getFields(), sb, ListChecker.NOOP_CHECKER); } else { @@ -665,6 +670,51 @@ private static boolean isParamNameCharacterValid(final char c) { return c > 32 && c <= 126 && c != '=' && c != ']' && c != '"'; } + /** + * Sanitizes an RFC 5424 {@code SD-ID}. + * + *

{@code SD-ID} uses the same {@code SD-NAME} production as {@code PARAM-NAME}, so invalid + * characters are replaced with {@code '?'}. The length is left alone, because + * {@link StructuredDataId} lets callers raise the 32 character limit.

+ * + * @param id the original structured data id + * @return a structured data id compliant with RFC 5424 + */ + private static String sanitizeSdId(final String id) { + return sanitizePrintableUsAscii(id, SD_NAME_EXCLUDED_CHARACTERS); + } + + /** + * Sanitizes an RFC 5424 {@code MSGID}. + * + *

{@code MSGID} is restricted to printable US-ASCII, so invalid characters are replaced with + * {@code '?'}.

+ * + * @param msgId the original message id + * @return a message id compliant with RFC 5424 + */ + private static String sanitizeMsgId(final String msgId) { + return sanitizePrintableUsAscii(msgId, Strings.EMPTY); + } + + private static String sanitizePrintableUsAscii(final String value, final String excluded) { + StringBuilder output = null; + for (int i = 0; i < value.length(); i++) { + final char cur = value.charAt(i); + if (cur > 32 && cur <= 126 && excluded.indexOf(cur) < 0) { + if (output != null) { + output.append(cur); + } + } else { + if (output == null) { + output = new StringBuilder(value.substring(0, i)); + } + output.append('?'); + } + } + return output != null ? output.toString() : value; + } + private String escapeParamValue(final String value) { StringBuilder output = null; for (int i = 0; i < value.length(); i++) { diff --git a/src/changelog/.2.x.x/fix_rfc5424_sd_id_and_msgid_sanitization.xml b/src/changelog/.2.x.x/fix_rfc5424_sd_id_and_msgid_sanitization.xml new file mode 100644 index 00000000000..656e2633082 --- /dev/null +++ b/src/changelog/.2.x.x/fix_rfc5424_sd_id_and_msgid_sanitization.xml @@ -0,0 +1,12 @@ + + + + + Sanitize the `SD-ID` and `MSGID` fields written by `Rfc5424Layout`, so that a `StructuredDataMessage` id or type cannot alter the structure of the emitted syslog record. + +