Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sympa MIME boundary lines can violate 70-character limit in RFC 2046 #1795

Open
adam12b1 opened this issue Jan 30, 2024 · 5 comments · May be fixed by #1809
Open

Sympa MIME boundary lines can violate 70-character limit in RFC 2046 #1795

adam12b1 opened this issue Jan 30, 2024 · 5 comments · May be fixed by #1809
Labels

Comments

@adam12b1
Copy link

adam12b1 commented Jan 30, 2024

Version

Sympa 6.2.72

Installation method

From ports on FreeBSD 13.2

Expected behavior

Sympa messages should be compliant.

Actual behavior

In a virtual robot with a long name, the Sympa-generated MIME boundary header on multipart messages can easily exceed 70 characters. That is the limit in the RFC, and if your email is processed through MailScanner, it gets blocked/quarantined/stripped with a complaint about a "Eudora long-mime-boundary attack" - but that is just one easy-to-see symptom. It occurs with MIME digests, or with moderation messages, or any other Sympa multipart MIME messages.

The heart of the problem is that these MIME boundary headers become non-compliant when they exceed 70 characters, and they should be truncated.

Steps to reproduce

Create a virtual domain with a name like "lists.a-very-long-domain-name.org", and a list within that domain, then try to submit a message for moderation or generate a MIME digest for the list, and look at the Content-Type: multipart/mixed; boundary="---------=1_<sympa.17... header. Or if you run MailScanner, you'll never see that header, you'll just see all content stripped and replaced with a warning.

Additional information

@adam12b1 adam12b1 added the bug label Jan 30, 2024
@adam12b1
Copy link
Author

And here is one way to fix the problem, simply truncating the $domain variable at 20 characters when it is inserted into the Message-ID, which is then used for the MIME boundary:

diff --git a/Sympa.pm b/Sympa.pm
index 24ee44b..d99773e 100644
--- a/Sympa.pm
+++ b/Sympa.pm
@@ -775,7 +775,7 @@ sub unique_message_id {
           (ref $that eq 'Sympa::List') ? $that->{'domain'}
         : ($that and $that ne '*') ? $that
         :                            $Conf::Conf{'domain'};
-    return sprintf '<sympa.%d.%d.%d.%d@%s>', $time, $usec, $PID,
+    return sprintf '<sympa.%d.%d.%d.%d@%.20s>', $time, $usec, $PID,
         (int rand 999), $domain;
 }

Any reason not to do that?

@adam12b1
Copy link
Author

...or perhaps this is better, not touching the Message-ID, only the MIME boundary:

diff --git a/Sympa/Message/Template.pm b/Sympa/Message/Template.pm
index a9ca7b6..44a450a 100644
--- a/Sympa/Message/Template.pm
+++ b/Sympa/Message/Template.pm
@@ -187,11 +187,11 @@ sub new {
         }
     }
     my $unique_id = Sympa::unique_message_id($robot_id);
-    $data->{'boundary'} = sprintf '----------=_%s', $unique_id
+    $data->{'boundary'} = sprintf '----------=_%0.55s', $unique_id
         unless $data->{'boundary'};
-    $data->{'boundary1'} = sprintf '---------=1_%s', $unique_id
+    $data->{'boundary1'} = sprintf '---------=1_%0.55s', $unique_id
         unless $data->{'boundary1'};
-    $data->{'boundary2'} = sprintf '---------=2_%s', $unique_id
+    $data->{'boundary2'} = sprintf '---------=2_%0.55s', $unique_id
         unless $data->{'boundary2'};

     my $self = $class->_new_from_template($that, $tpl . '.tt2',

@ikedas
Copy link
Member

ikedas commented Jan 31, 2024

I think

sprintf '<sympa.%d.%d.%d.%d@%.20s>', $time, $usec, $PID,
         (int rand 999), $domain;

would be better to be

sprintf '<sympa.%d.%d.%d.%d@%s>', $time, $usec, $PID,
         (int rand 999), substr $domain, -20;

so that uniqueness across domains will be ensured as much as possible.

@adam12b1
Copy link
Author

adam12b1 commented Feb 1, 2024

Oh sure, that is probably an even better way, thanks!

@ikedas
Copy link
Member

ikedas commented Feb 24, 2024

@adam12b1 , if possible, please check the patch in the PR above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants