[FIX] Prevent header injection with MIME4J DOM (#91)

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>

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() {

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 void testPublicConstructor() throws Exception {
         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 void testTabAfterDelimiter() throws Exception {
         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();
+    }
+
 }

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 void asBytesShouldSerializeTheMessage() throws Exception {
                 "\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);
+    }
 
 }