This is an automated email from the ASF dual-hosted git repository. chibenwa pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-jdkim.git
commit 754fc27b36c79c2dcb2ee84505c84c7b84a4ec95 Author: Benoit TELLIER <[email protected]> AuthorDate: Fri Oct 30 08:42:10 2026 +0100 [FIX] TagValue: restrict values to VALCHAR and the end of the list to FWS Values were matched with a negated class, so control and 8-bit characters were accepted, and the text after the final ";" was checked with trimFWS, which also skipped lone CRLFs: "a=b;\r\n\r\n\t" and "\r\n\r\n\r\n\t" both parsed. tval is now 1*VALCHAR (%x21-3A / %x3C-7E) as RFC 6376 section 3.2 requires, and only an FWS may follow the final ";". The Result test moves its control characters to an overriding record, since the parser no longer produces them. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../org/apache/james/jdkim/tagvalue/TagValue.java | 11 ++++++----- .../apache/james/jdkim/SignatureRecordImplTest.java | 2 +- .../java/org/apache/james/jdkim/api/ResultTest.java | 18 +++++++++++++++++- .../apache/james/jdkim/tagvalue/TagValueTest.java | 21 +++++++++++++++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/main/src/main/java/org/apache/james/jdkim/tagvalue/TagValue.java b/main/src/main/java/org/apache/james/jdkim/tagvalue/TagValue.java index cf48ae4..991a854 100644 --- a/main/src/main/java/org/apache/james/jdkim/tagvalue/TagValue.java +++ b/main/src/main/java/org/apache/james/jdkim/tagvalue/TagValue.java @@ -39,13 +39,16 @@ public class TagValue { private static final Pattern tagPattern = Pattern .compile("^[A-Za-z][A-Za-z0-9_]*$"); + // RFC 6376 section 3.2: tval = 1*VALCHAR, VALCHAR = %x21-3A / %x3C-7E // Use possessive matching to avoid heavy stack usage - private static final String tval = "[^; \t\r\n]++"; + private static final String tval = "[\\x21-\\x3a\\x3c-\\x7e]++"; // validate value chars // Use possessive matching to avoid heavy stack usage private static final Pattern valuePattern = Pattern.compile("^(?:" + tval + "(?:(?:(?:\r\n)?[\t ])++" + tval + ")*+)?$"); private static final Pattern fwsPattern = Pattern.compile("[ \t\r\n]"); + // FWS = [*WSP CRLF] 1*WSP + private static final Pattern trailingFWSPattern = Pattern.compile("^(?:(?:[ \t]*\r\n)?[ \t]+)?$"); // we may use a TreeMap because we may need to know original order. private final Map<String, CharSequence> tagValues; @@ -143,11 +146,9 @@ public class TagValue { for (int i = 0; i < data.length(); i++) { int equal = data.indexOf('=', i); if (equal == -1) { - // TODO check whether this is correct or not - // this allow FWS/WSP after the final ";" + // only FWS may follow the final ";" String rest = data.substring(i); - if (!rest.isEmpty() - && trimFWS(rest, 0, rest.length() - 1, true).length() > 0) { + if (!trailingFWSPattern.matcher(rest).matches()) { throw new IllegalStateException( "Unexpected termination at position " + i + ": " + data + " | [" + rest + "]"); diff --git a/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java b/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java index e75eb46..6d54bf4 100644 --- a/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java +++ b/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java @@ -258,7 +258,7 @@ public class SignatureRecordImplTest { @Test public void signatureWithCharactersOutsideTheBase64AlphabetShouldNotDecode() { SignatureRecordImpl record = new SignatureRecordImpl( - "v=1; a=rsa-sha256; d=example.com; s=s; h=from; bh=AA==; b=AAÿAA"); + "v=1; a=rsa-sha256; d=example.com; s=s; h=from; bh=AA==; b=AA.AA"); assertThatThrownBy(record::getSignature).isInstanceOf(IllegalArgumentException.class); } diff --git a/main/src/test/java/org/apache/james/jdkim/api/ResultTest.java b/main/src/test/java/org/apache/james/jdkim/api/ResultTest.java index 3115d4c..5272525 100644 --- a/main/src/test/java/org/apache/james/jdkim/api/ResultTest.java +++ b/main/src/test/java/org/apache/james/jdkim/api/ResultTest.java @@ -56,8 +56,24 @@ public class ResultTest { @Test public void headerTextShouldDropControlCharactersAndFolding() { + // the tag-list parser rejects control characters: other SignatureRecord implementations may not SignatureRecord record = new SignatureRecordImpl( - "v=1; a=rsa-sha256; d=(\u0001.example; s=se\r\n l\u007f; h=from; bh=AAAA; b=AAAA\r\n BBBB\r\n\tCCCC\u0001DDDD"); + "v=1; a=rsa-sha256; d=x.example; s=sel; h=from; bh=AAAA; b=AAAA") { + @Override + public CharSequence getDToken() { + return "(\u0001.example"; + } + + @Override + public CharSequence getSelector() { + return "se\r\n l\u007f"; + } + + @Override + public CharSequence getRawSignature() { + return "AAAA\r\n BBBB\r\n\tCCCC\u0001DDDD"; + } + }; Result result = new Result("bad \\ value\r\n \u007f\"", "", record, Result.Type.FAIL); assertThat(result.getHeaderTextWithReason()).isEqualTo( diff --git a/main/src/test/java/org/apache/james/jdkim/tagvalue/TagValueTest.java b/main/src/test/java/org/apache/james/jdkim/tagvalue/TagValueTest.java index e0ccd7e..6617b62 100644 --- a/main/src/test/java/org/apache/james/jdkim/tagvalue/TagValueTest.java +++ b/main/src/test/java/org/apache/james/jdkim/tagvalue/TagValueTest.java @@ -19,6 +19,7 @@ package org.apache.james.jdkim.tagvalue; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import org.junit.Assert; import static org.junit.Assert.assertTrue; import org.junit.Test; @@ -193,4 +194,24 @@ public class TagValueTest { return eq; } + @Test + public void valuesOutsideValcharShouldBeRejected() { + assertThatThrownBy(() -> new TagValue("a=\u000b")).isInstanceOf(IllegalStateException.class); + assertThatThrownBy(() -> new TagValue("a=\u0090")).isInstanceOf(IllegalStateException.class); + assertThatThrownBy(() -> new TagValue("a=b\u0000c")).isInstanceOf(IllegalStateException.class); + } + + @Test + public void trailingWhitespaceThatIsNotFwsShouldBeRejected() { + assertThatThrownBy(() -> new TagValue("a=b;\r\n\r\n\t")).isInstanceOf(IllegalStateException.class); + assertThatThrownBy(() -> new TagValue("\r\n\r\n\r\n\t")).isInstanceOf(IllegalStateException.class); + assertThatThrownBy(() -> new TagValue("a=b;\r\n")).isInstanceOf(IllegalStateException.class); + } + + @Test + public void trailingFwsShouldBeAccepted() { + new TagValue("a=b; "); + new TagValue("a=b; \r\n\t"); + new TagValue("a=b;\r\n "); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
