160 lines
5.9 KiB
Diff
160 lines
5.9 KiB
Diff
From 9dec5df2a588fed8027839815daefa79ee66efd1 Mon Sep 17 00:00:00 2001
|
|
From: Benoit TELLIER <btellier@linagora.com>
|
|
Date: Fri, 5 Jan 2024 08:12:54 +0100
|
|
Subject: [PATCH] [FIX] Prevent header injection with MIME4J DOM (#91)
|
|
|
|
---
|
|
core/pom.xml | 5 +++
|
|
.../apache/james/mime4j/stream/RawField.java | 23 +++++++++++
|
|
.../james/mime4j/stream/RawFieldTest.java | 40 +++++++++++++++++--
|
|
.../message/DefaultMessageWriterTest.java | 11 +++++
|
|
4 files changed, 76 insertions(+), 3 deletions(-)
|
|
|
|
diff --git a/core/pom.xml b/core/pom.xml
|
|
index 942e3e3..412ad67 100644
|
|
--- a/core/pom.xml
|
|
+++ b/core/pom.xml
|
|
@@ -42,6 +42,11 @@
|
|
<artifactId>commons-io</artifactId>
|
|
<scope>test</scope>
|
|
</dependency>
|
|
+ <dependency>
|
|
+ <groupId>org.assertj</groupId>
|
|
+ <artifactId>assertj-core</artifactId>
|
|
+ <scope>test</scope>
|
|
+ </dependency>
|
|
</dependencies>
|
|
|
|
<build>
|
|
diff --git a/core/src/main/java/org/apache/james/mime4j/stream/RawField.java b/core/src/main/java/org/apache/james/mime4j/stream/RawField.java
|
|
index 8bcaa77..03a00dd 100644
|
|
--- a/core/src/main/java/org/apache/james/mime4j/stream/RawField.java
|
|
+++ b/core/src/main/java/org/apache/james/mime4j/stream/RawField.java
|
|
@@ -55,6 +55,29 @@ public final class RawField implements Field {
|
|
|
|
public RawField(String name, String body) {
|
|
this(null, -1, name, body);
|
|
+
|
|
+ int pos = 0;
|
|
+
|
|
+ while (true) {
|
|
+ pos = body.indexOf('\r', pos);
|
|
+ if (pos < 0) {
|
|
+ break;
|
|
+ }
|
|
+ if (pos < body.length() + 2) {
|
|
+ if (body.charAt(pos + 1) != '\n') {
|
|
+ throw new IllegalArgumentException("Injection of un-encoded line breaks inside header field could be assimilated to header injection");
|
|
+ }
|
|
+ if (pos != body.length() - 2 && !isSpace(body, pos + 2)) {
|
|
+ throw new IllegalArgumentException("Injection of un-encoded line breaks inside header field could be assimilated to header injection");
|
|
+ }
|
|
+ }
|
|
+ pos ++;
|
|
+ }
|
|
+ }
|
|
+
|
|
+ private static boolean isSpace(String body, int pos) {
|
|
+ return body.charAt(pos) == ' '
|
|
+ || body.charAt(pos) == '\t';
|
|
}
|
|
|
|
public ByteSequence getRaw() {
|
|
diff --git a/core/src/test/java/org/apache/james/mime4j/stream/RawFieldTest.java b/core/src/test/java/org/apache/james/mime4j/stream/RawFieldTest.java
|
|
index 5a1cc7d..90d8513 100644
|
|
--- a/core/src/test/java/org/apache/james/mime4j/stream/RawFieldTest.java
|
|
+++ b/core/src/test/java/org/apache/james/mime4j/stream/RawFieldTest.java
|
|
@@ -19,6 +19,9 @@
|
|
|
|
package org.apache.james.mime4j.stream;
|
|
|
|
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode;
|
|
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
|
|
+
|
|
import junit.framework.Assert;
|
|
import org.apache.james.mime4j.util.ByteSequence;
|
|
import org.apache.james.mime4j.util.ContentUtil;
|
|
@@ -45,11 +48,11 @@ public class RawFieldTest {
|
|
Assert.assertEquals("stuff", field1.getBody());
|
|
Assert.assertEquals("raw: stuff", field1.toString());
|
|
|
|
- RawField field2 = new RawField("raw", null);
|
|
+ RawField field2 = new RawField("raw", "any");
|
|
Assert.assertNull(field2.getRaw());
|
|
Assert.assertEquals("raw", field2.getName());
|
|
- Assert.assertEquals(null, field2.getBody());
|
|
- Assert.assertEquals("raw: ", field2.toString());
|
|
+ Assert.assertEquals("any", field2.getBody());
|
|
+ Assert.assertEquals("raw: any", field2.toString());
|
|
}
|
|
|
|
@Test
|
|
@@ -63,4 +66,35 @@ public class RawFieldTest {
|
|
Assert.assertEquals(s, field.toString());
|
|
}
|
|
|
|
+ @Test
|
|
+ public void shouldRejectAmbiguousLineEnding() {
|
|
+ assertThatThrownBy(() -> new RawField("Name", "Value\r\ncheating")).isInstanceOf(IllegalArgumentException.class);
|
|
+ }
|
|
+
|
|
+ @Test
|
|
+ public void shouldAcceptCRLFTerminatedHeader() {
|
|
+ assertThatCode(() -> new RawField("Name", "Value\r\n")).doesNotThrowAnyException();
|
|
+ }
|
|
+
|
|
+ @Test
|
|
+ public void shouldAcceptTabFolding() {
|
|
+ assertThatCode(() -> new RawField("Name", "Value\r\n\thello")).doesNotThrowAnyException();
|
|
+ }
|
|
+
|
|
+ @Test
|
|
+ public void shouldAcceptSpaceFolding() {
|
|
+ assertThatCode(() -> new RawField("Name", "Value\r\n hello")).doesNotThrowAnyException();
|
|
+ }
|
|
+
|
|
+ @Test
|
|
+ public void shouldAcceptOnlyDelimiter() {
|
|
+ assertThatCode(() -> new RawField("Name", "\r\n")).doesNotThrowAnyException();
|
|
+ }
|
|
+
|
|
+
|
|
+ @Test
|
|
+ public void shouldAcceptNoDelimiter() {
|
|
+ assertThatCode(() -> new RawField("Name", "Value")).doesNotThrowAnyException();
|
|
+ }
|
|
+
|
|
}
|
|
diff --git a/dom/src/test/java/org/apache/james/mime4j/message/DefaultMessageWriterTest.java b/dom/src/test/java/org/apache/james/mime4j/message/DefaultMessageWriterTest.java
|
|
index dece2b5..19eafe2 100644
|
|
--- a/dom/src/test/java/org/apache/james/mime4j/message/DefaultMessageWriterTest.java
|
|
+++ b/dom/src/test/java/org/apache/james/mime4j/message/DefaultMessageWriterTest.java
|
|
@@ -20,6 +20,7 @@
|
|
package org.apache.james.mime4j.message;
|
|
|
|
import static org.assertj.core.api.Assertions.assertThat;
|
|
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
|
|
|
import org.apache.james.mime4j.Charsets;
|
|
import org.apache.james.mime4j.dom.Message;
|
|
@@ -46,5 +47,15 @@ public class DefaultMessageWriterTest {
|
|
"\r\n" +
|
|
"this is the body");
|
|
}
|
|
+
|
|
+ @Test
|
|
+ public void shouldThrowOnHeaderInjectionAttempt() throws Exception {
|
|
+ Message.Builder builder = Message.Builder.of()
|
|
+ .setBody("this is the body", Charsets.UTF_8)
|
|
+ .setFrom("sender@localhost");
|
|
+
|
|
+ assertThatThrownBy(() -> builder.setContentTransferEncoding("victim@attacker.com\r\nReply-To: attacker@evil.com"))
|
|
+ .isInstanceOf(IllegalArgumentException.class);
|
|
+ }
|
|
|
|
}
|
|
\ No newline at end of file
|
|
--
|
|
2.33.0
|
|
|