From 7e4c807db79ed3ffe3c3b6f0fe2d605a97108fb9 Mon Sep 17 00:00:00 2001 From: Jeffrey Stedfast Date: Wed, 21 Aug 2019 19:57:50 -0400 Subject: [PATCH] Fixed MimeParser/MimeMessage/HeaderList to deal with invalid headers better Partial fix for issue #499 --- MimeKit/Cryptography/DkimVerifierBase.cs | 2 +- MimeKit/Header.cs | 47 +++++++++++----- MimeKit/HeaderList.cs | 22 +++++--- MimeKit/MimeMessage.cs | 22 +++++--- MimeKit/MimeParser.cs | 9 +--- UnitTests/MimeMessageTests.cs | 69 ++++++++++++++++++++++++ 6 files changed, 132 insertions(+), 39 deletions(-) diff --git a/MimeKit/Cryptography/DkimVerifierBase.cs b/MimeKit/Cryptography/DkimVerifierBase.cs index cc463c3d6d..53651ed5ac 100644 --- a/MimeKit/Cryptography/DkimVerifierBase.cs +++ b/MimeKit/Cryptography/DkimVerifierBase.cs @@ -493,7 +493,7 @@ internal static Header GetSignedSignatureHeader (Header header) Array.Resize (ref rawValue, length); - return new Header (header.Options, header.RawField, rawValue); + return new Header (header.Options, header.RawField, rawValue, false); } } } diff --git a/MimeKit/Header.cs b/MimeKit/Header.cs index 806ae6d7fd..488ccdb360 100644 --- a/MimeKit/Header.cs +++ b/MimeKit/Header.cs @@ -291,12 +291,12 @@ internal Header (ParserOptions options, HeaderId id, string name, byte[] field, Id = id; } - internal Header (ParserOptions options, byte[] field, byte[] value) + internal Header (ParserOptions options, byte[] field, byte[] value, bool invalid) { var chars = new char[field.Length]; int count = 0; - while (count < field.Length && !field[count].IsBlank ()) { + while (count < field.Length && (invalid || !field[count].IsBlank ())) { chars[count] = (char) field[count]; count++; } @@ -307,6 +307,7 @@ internal Header (ParserOptions options, byte[] field, byte[] value) Field = new string (chars, 0, count); Id = Field.ToHeaderId (); + IsInvalid = invalid; } internal Header (ParserOptions options, HeaderId id, string field, byte[] value) @@ -327,7 +328,9 @@ internal Header (ParserOptions options, HeaderId id, string field, byte[] value) /// A copy of the header with its current state. public Header Clone () { - var header = new Header (Options, Id, Field, RawField, RawValue); + var header = new Header (Options, Id, Field, rawField, rawValue) { + IsInvalid = IsInvalid + }; // if the textValue has already been calculated, set it on the cloned header as well. header.textValue = textValue; @@ -369,6 +372,10 @@ public HeaderId Id { get; private set; } + internal bool IsInvalid { + get; private set; + } + /// /// Gets the raw field name of the header. /// @@ -1177,7 +1184,7 @@ void OnChanged () /// A string representing the . public override string ToString () { - return Field + ": " + Value; + return IsInvalid ? Field : Field + ": " + Value; } /// @@ -1250,6 +1257,7 @@ internal static unsafe bool TryParse (ParserOptions options, byte* input, int le byte* inend = input + length; byte* start = input; byte* inptr = input; + var invalid = false; // find the end of the field name if (strict) { @@ -1264,8 +1272,13 @@ internal static unsafe bool TryParse (ParserOptions options, byte* input, int le inptr++; if (inptr == inend || *inptr != ':') { - header = null; - return false; + if (strict) { + header = null; + return false; + } + + invalid = true; + inptr = inend; } var field = new byte[(int) (inptr - start)]; @@ -1276,19 +1289,25 @@ internal static unsafe bool TryParse (ParserOptions options, byte* input, int le *outptr++ = *start++; } - inptr++; + byte[] value; - int count = (int) (inend - inptr); - var value = new byte[count]; + if (inptr < inend) { + inptr++; - fixed (byte *outbuf = value) { - byte* outptr = outbuf; + int count = (int) (inend - inptr); + value = new byte[count]; + + fixed (byte* outbuf = value) { + byte* outptr = outbuf; - while (inptr < inend) - *outptr++ = *inptr++; + while (inptr < inend) + *outptr++ = *inptr++; + } + } else { + value = new byte[0]; } - header = new Header (options, field, value); + header = new Header (options, field, value, invalid); return true; } diff --git a/MimeKit/HeaderList.cs b/MimeKit/HeaderList.cs index c02bc82eba..72ac40f891 100644 --- a/MimeKit/HeaderList.cs +++ b/MimeKit/HeaderList.cs @@ -699,11 +699,14 @@ public string this [string field] { filtered.Add (options.CreateNewLineFilter ()); foreach (var header in headers) { - var rawValue = header.GetRawValue (options); - filtered.Write (header.RawField, 0, header.RawField.Length, cancellationToken); - filtered.Write (Header.Colon, 0, Header.Colon.Length, cancellationToken); - filtered.Write (rawValue, 0, rawValue.Length, cancellationToken); + + if (!header.IsInvalid) { + var rawValue = header.GetRawValue (options); + + filtered.Write (Header.Colon, 0, Header.Colon.Length, cancellationToken); + filtered.Write (rawValue, 0, rawValue.Length, cancellationToken); + } } filtered.Flush (cancellationToken); @@ -752,11 +755,14 @@ public string this [string field] { filtered.Add (options.CreateNewLineFilter ()); foreach (var header in headers) { - var rawValue = header.GetRawValue (options); - await filtered.WriteAsync (header.RawField, 0, header.RawField.Length, cancellationToken).ConfigureAwait (false); - await filtered.WriteAsync (Header.Colon, 0, Header.Colon.Length, cancellationToken).ConfigureAwait (false); - await filtered.WriteAsync (rawValue, 0, rawValue.Length, cancellationToken).ConfigureAwait (false); + + if (!header.IsInvalid) { + var rawValue = header.GetRawValue (options); + + await filtered.WriteAsync (Header.Colon, 0, Header.Colon.Length, cancellationToken).ConfigureAwait (false); + await filtered.WriteAsync (rawValue, 0, rawValue.Length, cancellationToken).ConfigureAwait (false); + } } await filtered.FlushAsync (cancellationToken).ConfigureAwait (false); diff --git a/MimeKit/MimeMessage.cs b/MimeKit/MimeMessage.cs index b55eb3bfc8..ead8e23f28 100644 --- a/MimeKit/MimeMessage.cs +++ b/MimeKit/MimeMessage.cs @@ -1094,11 +1094,14 @@ public virtual void Prepare (EncodingConstraint constraint, int maxLineLength = if (options.HiddenHeaders.Contains (header.Id)) continue; - var rawValue = header.GetRawValue (options); - filtered.Write (header.RawField, 0, header.RawField.Length, cancellationToken); - filtered.Write (Header.Colon, 0, Header.Colon.Length, cancellationToken); - filtered.Write (rawValue, 0, rawValue.Length, cancellationToken); + + if (!header.IsInvalid) { + var rawValue = header.GetRawValue (options); + + filtered.Write (Header.Colon, 0, Header.Colon.Length, cancellationToken); + filtered.Write (rawValue, 0, rawValue.Length, cancellationToken); + } } filtered.Flush (cancellationToken); @@ -1167,11 +1170,14 @@ public virtual void Prepare (EncodingConstraint constraint, int maxLineLength = if (options.HiddenHeaders.Contains (header.Id)) continue; - var rawValue = header.GetRawValue (options); - await filtered.WriteAsync (header.RawField, 0, header.RawField.Length, cancellationToken).ConfigureAwait (false); - await filtered.WriteAsync (Header.Colon, 0, Header.Colon.Length, cancellationToken).ConfigureAwait (false); - await filtered.WriteAsync (rawValue, 0, rawValue.Length, cancellationToken).ConfigureAwait (false); + + if (!header.IsInvalid) { + var rawValue = header.GetRawValue (options); + + await filtered.WriteAsync (Header.Colon, 0, Header.Colon.Length, cancellationToken).ConfigureAwait (false); + await filtered.WriteAsync (rawValue, 0, rawValue.Length, cancellationToken).ConfigureAwait (false); + } } await filtered.FlushAsync (cancellationToken).ConfigureAwait (false); diff --git a/MimeKit/MimeParser.cs b/MimeKit/MimeParser.cs index ed9d99aef3..7fce8f4a50 100644 --- a/MimeKit/MimeParser.cs +++ b/MimeKit/MimeParser.cs @@ -681,14 +681,7 @@ unsafe void ParseAndAppendHeader () fixed (byte* buf = headerBuffer) { Header header; - if (!Header.TryParse (options, buf, headerIndex, false, out header)) { -#if DEBUG - Debug.WriteLine (string.Format ("Invalid header at offset {0}: {1}", headerOffset, ConvertToCString (headerBuffer, 0, headerIndex))); -#endif - headerIndex = 0; - return; - } - + Header.TryParse (options, buf, headerIndex, false, out header); header.Offset = headerOffset; headers.Add (header); headerIndex = 0; diff --git a/UnitTests/MimeMessageTests.cs b/UnitTests/MimeMessageTests.cs index aabfbb4aea..da5864cf01 100644 --- a/UnitTests/MimeMessageTests.cs +++ b/UnitTests/MimeMessageTests.cs @@ -538,6 +538,75 @@ This is the preamble. } } + [Test] + public async Task TestReserializationInvalidHeaders () + { + string rawMessageText = @"From: Example Test +MIME-Version: 1.0 +Content-Type: multipart/mixed; + boundary=""simple boundary"" +Example: test +Test +Test Test +Test: +Test: +Test: Test +Test Example: + +This is the preamble. + +--simple boundary +Content-TypeS: text/plain + +This is a test. + +--simple boundary +Content-Type: text/plain; +Content-Disposition: attachment; +Content-Transfer-Encoding: test; +Content-Transfer-Encoding: binary; +Test Test Test: Test Test +Te$t($)*$= Test Test: Abc def +test test = test +test test :: test +filename=""test.txt"" + +Another test. + +--simple boundary-- + + +This is the epilogue. +".Replace ("\r\n", "\n"); + + using (var source = new MemoryStream (Encoding.UTF8.GetBytes (rawMessageText))) { + var parser = new MimeParser (source, MimeFormat.Default); + var message = parser.ParseMessage (); + + using (var serialized = new MemoryStream ()) { + var options = FormatOptions.Default.Clone (); + options.NewLineFormat = NewLineFormat.Unix; + + message.WriteTo (options, serialized); + + var result = Encoding.UTF8.GetString (serialized.ToArray ()); + + Assert.AreEqual (rawMessageText, result, "Reserialized message is not identical to the original."); + } + + using (var serialized = new MemoryStream ()) { + var options = FormatOptions.Default.Clone (); + options.NewLineFormat = NewLineFormat.Unix; + + await message.WriteToAsync (options, serialized); + + var result = Encoding.UTF8.GetString (serialized.ToArray ()); + + Assert.AreEqual (rawMessageText, result, "Reserialized (async) message is not identical to the original."); + } + } + } + [Test] public void TestMailMessageToMimeMessage () {