This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-mime4j.git
The following commit(s) were added to refs/heads/master by this push:
new 9dec5df2 [FIX] Prevent header injection with MIME4J DOM (#91)
9dec5df2 is described below
commit 9dec5df2a588fed8027839815daefa79ee66efd1
Author: Benoit TELLIER <btell...@linagora.com>
AuthorDate: Fri Jan 5 08:12:54 2024 +0100
[FIX] Prevent header injection with MIME4J DOM (#91)
---
core/pom.xml | 5 +++
.../org/apache/james/mime4j/stream/RawField.java | 23 +++++++++++++
.../apache/james/mime4j/stream/RawFieldTest.java | 40 ++++++++++++++++++++--
.../mime4j/message/DefaultMessageWriterTest.java | 11 ++++++
4 files changed, 76 insertions(+), 3 deletions(-)
diff --git a/core/pom.xml b/core/pom.xml
index 93d7827b..f0fba5dd 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -42,6 +42,11 @@
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
</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 e64c6a73..1be89662 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
@@ -56,6 +56,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 5a1cc7da..90d85134 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 dece2b50..19eafe24 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("vic...@attacker.com\r\nReply-To:
attac...@evil.com"))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
