Fixed MimeReader to better handle garbage at the start of an mbox

jstedfast · Dec 20, 2024 · b83f08e · b83f08e
1 parent c9fb107
commit b83f08e
Show file tree

Hide file tree

Showing 4 changed files with 216 additions and 35 deletions.
diff --git a/MimeKit/AsyncMimeReader.cs b/MimeKit/AsyncMimeReader.cs
@@ -75,14 +75,15 @@ async Task<bool> StepByteOrderMarkAsync (CancellationToken cancellationToken)
 		async Task StepMboxMarkerAsync (CancellationToken cancellationToken)
 		{
 			int mboxMarkerIndex, mboxMarkerLength;
-			long mboxMarkerOffset;
+			bool midline = false;
 			bool complete;
 			int left = 0;
 
+			// consume data until we find a line that begins with "From "
 			do {
-				var available = await ReadAheadAsync (Math.Max (ReadAheadSize, left), 0, cancellationToken).ConfigureAwait (false);
+				var available = await ReadAheadAsync (5, 0, cancellationToken).ConfigureAwait (false);
 
-				if (available <= left) {
+				if (available < 5) {
 					// failed to find a From line; EOF reached
 					state = MimeParserState.Error;
 					inputIndex = inputEnd;
@@ -91,7 +92,27 @@ async Task StepMboxMarkerAsync (CancellationToken cancellationToken)
 
 				unsafe {
 					fixed (byte* inbuf = input) {
-						complete = StepMboxMarker (inbuf, ref left, out mboxMarkerIndex, out mboxMarkerLength, out mboxMarkerOffset);
+						complete = StepMboxMarkerStart (inbuf, ref midline);
+					}
+				}
+			} while (!complete);
+
+			var mboxMarkerOffset = GetOffset (inputIndex);
+
+			// FIXME: if the mbox marker is > the size of the input buffer, parsing will fail
+			do {
+				var available = await ReadAheadAsync (Math.Max (ReadAheadSize, left + 1), 0, cancellationToken).ConfigureAwait (false);
+
+				if (available <= left) {
+					// failed to find the end of the mbox marker; EOF reached
+					state = MimeParserState.Error;
+					inputIndex = inputEnd;
+					return;
+				}
+
+				unsafe {
+					fixed (byte* inbuf = input) {
+						complete = StepMboxMarker (inbuf, ref left, out mboxMarkerIndex, out mboxMarkerLength);
 					}
 				}
 			} while (!complete);

diff --git a/MimeKit/MimeReader.cs b/MimeKit/MimeReader.cs
@@ -1203,77 +1203,133 @@ static unsafe bool IsMboxMarker (byte[] text, bool allowMunged = false)
 			}
 		}
 
-		unsafe bool StepMboxMarker (byte* inbuf, ref int left, out int mboxMarkerIndex, out int mboxMarkerLength, out long mboxMarkerOffset)
+		unsafe bool StepMboxMarkerStart (byte* inbuf, ref bool midline)
 		{
 			byte* inptr = inbuf + inputIndex;
 			byte* inend = inbuf + inputEnd;
 
 			*inend = (byte) '\n';
 
-			while (inptr < inend) {
-				int startIndex = inputIndex;
-				byte* start = inptr;
-
-				// scan for the end of the line
+			if (midline) {
+				// we're in the middle of a line, so we need to scan for the end of the line
 				while (*inptr != (byte) '\n')
 					inptr++;
 
 				if (inptr == inend) {
 					// we don't have enough input data
-					left = (int) (inptr - start);
-					mboxMarkerOffset = 0;
-					mboxMarkerLength = 0;
-					mboxMarkerIndex = 0;
+					inputIndex = inputEnd;
 					return false;
 				}
 
-				var markerLength = (int) (inptr - start);
-
-				if (inptr > start && *(inptr - 1) == (byte) '\r')
-					markerLength--;
-
 				// consume the '\n'
 				inptr++;
-
-				var lineLength = (int) (inptr - start);
-
-				inputIndex += lineLength;
+				inputIndex = (int) (inptr - inbuf);
 				IncrementLineNumber (inputIndex);
+				midline = false;
+			}
 
-				if (markerLength >= 5 && IsMboxMarker (start)) {
-					mboxMarkerOffset = GetOffset (startIndex);
-					mboxMarkerLength = markerLength;
-					mboxMarkerIndex = startIndex;
+			while (inptr + 5 < inend) {
+				if (IsMboxMarker (inptr)) {
+					// we have found the start of the mbox marker
 					return true;
 				}
+
+				// scan for the end of the line
+				while (*inptr != (byte) '\n')
+					inptr++;
+
+				if (inptr == inend) {
+					// we don't have enough data to check for a From line
+					midline = true;
+					break;
+				}
+
+				// consume the '\n'
+				inptr++;
+				inputIndex = (int) (inptr - inbuf);
+				IncrementLineNumber (inputIndex);
 			}
 
-			mboxMarkerOffset = 0;
-			mboxMarkerLength = 0;
-			mboxMarkerIndex = 0;
-			left = 0;
+			inputIndex = (int) (inptr - inbuf);
 
 			return false;
 		}
 
+		unsafe bool StepMboxMarker (byte* inbuf, ref int left, out int mboxMarkerIndex, out int mboxMarkerLength)
+		{
+			byte* inptr = inbuf + inputIndex;
+			byte* inend = inbuf + inputEnd;
+			int startIndex = inputIndex;
+			byte* start = inptr;
+
+			*inend = (byte) '\n';
+
+			// scan for the end of the line
+			while (*inptr != (byte) '\n')
+				inptr++;
+
+			if (inptr == inend) {
+				// we don't have enough input data
+				left = (int) (inptr - start);
+				mboxMarkerLength = 0;
+				mboxMarkerIndex = 0;
+				return false;
+			}
+
+			var markerLength = (int) (inptr - start);
+
+			if (inptr > start && *(inptr - 1) == (byte) '\r')
+				markerLength--;
+
+			// consume the '\n'
+			inptr++;
+
+			var lineLength = (int) (inptr - start);
+
+			inputIndex += lineLength;
+			IncrementLineNumber (inputIndex);
+
+			mboxMarkerLength = markerLength;
+			mboxMarkerIndex = startIndex;
+
+			return true;
+		}
+
 		unsafe void StepMboxMarker (byte* inbuf, CancellationToken cancellationToken)
 		{
 			int mboxMarkerIndex, mboxMarkerLength;
-			long mboxMarkerOffset;
+			bool midline = false;
 			bool complete;
 			int left = 0;
 
+			// consume data until we find a line that begins with "From "
 			do {
-				var available = ReadAhead (Math.Max (ReadAheadSize, left), 0, cancellationToken);
+				var available = ReadAhead (5, 0, cancellationToken);
 
-				if (available <= left) {
+				if (available < 5) {
 					// failed to find a From line; EOF reached
 					state = MimeParserState.Error;
 					inputIndex = inputEnd;
 					return;
 				}
 
-				complete = StepMboxMarker (inbuf, ref left, out mboxMarkerIndex, out mboxMarkerLength, out mboxMarkerOffset);
+				complete = StepMboxMarkerStart (inbuf, ref midline);
+			} while (!complete);
+
+			var mboxMarkerOffset = GetOffset (inputIndex);
+
+			// FIXME: if the mbox marker is > the size of the input buffer, parsing will fail
+			do {
+				var available = ReadAhead (Math.Max (ReadAheadSize, left + 1), 0, cancellationToken);
+
+				if (available <= left) {
+					// failed to find the end of the mbox marker; EOF reached
+					state = MimeParserState.Error;
+					inputIndex = inputEnd;
+					return;
+				}
+
+				complete = StepMboxMarker (inbuf, ref left, out mboxMarkerIndex, out mboxMarkerLength);
 			} while (!complete);
 
 			OnMboxMarkerRead (input, mboxMarkerIndex, mboxMarkerLength, mboxMarkerOffset, lineNumber - 1, cancellationToken);

diff --git a/UnitTests/ExperimentalMimeParserTests.cs b/UnitTests/ExperimentalMimeParserTests.cs
@@ -2763,6 +2763,58 @@ public async Task TestSimpleMboxWithByteOrderMarkAsync ()
 			}
 		}
 
+		[TestCase (80)]
+		[TestCase (4094)] // tests the not-enough-data code-path in MimeReader.StepMboxMarker()
+		[TestCase (4096)]
+		[TestCase (4097)] // tests midline code-paths in MimeReader.StepMboxMarkerStart()
+		[TestCase (8193)] // tests the not-enough-data midline code-path in MimeReader.StepMboxMarkerStart()
+		public void TestSimpleMboxWithGarbageBeforeMboxMarker (int garbageLength)
+		{
+			var garbage = new byte[garbageLength];
+
+			for (int i = 0; i < garbageLength - 2; i++)
+				garbage[i] = (byte) 'X';
+			garbage[garbage.Length - 2] = (byte) '\r';
+			garbage[garbage.Length - 1] = (byte) '\n';
+
+			using (var stream = new MemoryStream ()) {
+				stream.Write (garbage, 0, garbage.Length);
+
+				using (var file = File.OpenRead (Path.Combine (MboxDataDir, "simple.mbox.txt")))
+					file.CopyTo (stream, 4096);
+
+				stream.Position = 0;
+
+				AssertSimpleMbox (stream);
+			}
+		}
+
+		[TestCase (80)]
+		[TestCase (4094)] // tests the not-enough-data code-path in MimeReader.StepMboxMarker()
+		[TestCase (4096)]
+		[TestCase (4097)] // tests midline code-paths in MimeReader.StepMboxMarkerStart()
+		[TestCase (8193)] // tests the not-enough-data midline code-path in MimeReader.StepMboxMarkerStart()
+		public async Task TestSimpleMboxWithGarbageBeforeMboxMarkerAsync (int garbageLength)
+		{
+			var garbage = new byte[1024];
+
+			for (int i = 0; i < garbage.Length - 2; i++)
+				garbage[i] = (byte) 'X';
+			garbage[garbage.Length - 2] = (byte) '\r';
+			garbage[garbage.Length - 1] = (byte) '\n';
+
+			using (var stream = new MemoryStream ()) {
+				stream.Write (garbage, 0, garbage.Length);
+
+				using (var file = File.OpenRead (Path.Combine (MboxDataDir, "simple.mbox.txt")))
+					file.CopyTo (stream, 4096);
+
+				stream.Position = 0;
+
+				await AssertSimpleMboxAsync (stream);
+			}
+		}
+
 		static void DumpMimeTree (StringBuilder builder, MimeEntity entity, int depth)
 		{
 			if (depth > 0)

diff --git a/UnitTests/MimeParserTests.cs b/UnitTests/MimeParserTests.cs
@@ -2747,6 +2747,58 @@ public async Task TestSimpleMboxWithByteOrderMarkAsync ()
 			}
 		}
 
+		[TestCase (80)]
+		[TestCase (4094)]
+		[TestCase (4096)]
+		//[TestCase (4097)]
+		//[TestCase (8193)]
+		public void TestSimpleMboxWithGarbageBeforeMboxMarker (int garbageLength)
+		{
+			var garbage = new byte[garbageLength];
+
+			for (int i = 0; i < garbageLength - 2; i++)
+				garbage[i] = (byte) 'X';
+			garbage[garbage.Length - 2] = (byte) '\r';
+			garbage[garbage.Length - 1] = (byte) '\n';
+
+			using (var stream = new MemoryStream ()) {
+				stream.Write (garbage, 0, garbage.Length);
+
+				using (var file = File.OpenRead (Path.Combine (MboxDataDir, "simple.mbox.txt")))
+					file.CopyTo (stream, 4096);
+
+				stream.Position = 0;
+
+				AssertSimpleMbox (stream);
+			}
+		}
+
+		[TestCase (80)]
+		[TestCase (4094)]
+		[TestCase (4096)]
+		//[TestCase (4097)]
+		//[TestCase (8193)]
+		public async Task TestSimpleMboxWithGarbageBeforeMboxMarkerAsync (int garbageLength)
+		{
+			var garbage = new byte[1024];
+
+			for (int i = 0; i < garbage.Length - 2; i++)
+				garbage[i] = (byte) 'X';
+			garbage[garbage.Length - 2] = (byte) '\r';
+			garbage[garbage.Length - 1] = (byte) '\n';
+
+			using (var stream = new MemoryStream ()) {
+				stream.Write (garbage, 0, garbage.Length);
+
+				using (var file = File.OpenRead (Path.Combine (MboxDataDir, "simple.mbox.txt")))
+					file.CopyTo (stream, 4096);
+
+				stream.Position = 0;
+
+				await AssertSimpleMboxAsync (stream);
+			}
+		}
+
 		static void DumpMimeTree (StringBuilder builder, MimeEntity entity, int depth)
 		{
 			if (depth > 0)