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