Browse code

Better handling of Gibe.3 boundary exploit

git-svn: trunk@653

Nigel Horne authored on 2004/07/06 18:32:45
Showing 1 changed files
... ...
@@ -17,6 +17,9 @@
17 17
  *
18 18
  * Change History:
19 19
  * $Log: mbox.c,v $
20
+ * Revision 1.86  2004/07/06 09:32:45  nigelhorne
21
+ * Better handling of Gibe.3 boundary exploit
22
+ *
20 23
  * Revision 1.85  2004/06/30 19:48:58  nigelhorne
21 24
  * Some TR.Happy99.SKA were getting through
22 25
  *
... ...
@@ -243,7 +246,7 @@
243 243
  * Compilable under SCO; removed duplicate code with message.c
244 244
  *
245 245
  */
246
-static	char	const	rcsid[] = "$Id: mbox.c,v 1.85 2004/06/30 19:48:58 nigelhorne Exp $";
246
+static	char	const	rcsid[] = "$Id: mbox.c,v 1.86 2004/07/06 09:32:45 nigelhorne Exp $";
247 247
 
248 248
 #if HAVE_CONFIG_H
249 249
 #include "clamav-config.h"
... ...
@@ -656,7 +659,8 @@ parseEmailHeaders(message *m, const table_t *rfc821Table, bool destroy)
656 656
 				free(buffer);
657 657
 		} else {
658 658
 			/*cli_dbgmsg("Add line to body '%s'\n", buffer);*/
659
-			messageAddLine(ret, buffer, 0);
659
+			if(messageAddLine(ret, buffer, 0) < 0)
660
+				break;
660 661
 		}
661 662
 	}
662 663
 
... ...
@@ -1742,24 +1746,34 @@ parseEmailBody(message *messageIn, blob **blobsIn, int nBlobs, text *textIn, con
1742 1742
 static int
1743 1743
 boundaryStart(const char *line, const char *boundary)
1744 1744
 {
1745
+	/*cli_dbgmsg("boundaryStart: line = '%s' boundary = '%s'\n", line, boundary);*/
1746
+	if(line == NULL)
1747
+		return 0;	/* empty line */
1748
+
1749
+	if(*line++ != '-')
1750
+		return 0;
1751
+
1745 1752
 	/*
1746
-	 * Gibe.B3 is broken it has:
1753
+	 * Gibe.B3 is broken, it has:
1747 1754
 	 *	boundary="---- =_NextPart_000_01C31177.9DC7C000"
1748 1755
 	 * but it's boundaries look like
1749 1756
 	 *	------ =_NextPart_000_01C31177.9DC7C000
1750
-	 * notice the extra '-'
1757
+	 * notice the one too few '-'.
1758
+	 * Presumably this is a deliberate exploitation of a bug in some mail
1759
+	 * clients.
1760
+	 *
1761
+	 * The trouble is that this creates a lot of false positives for
1762
+	 * boundary conditions, if we're too lax about matches. We do our level
1763
+	 * best to avoid these false positives. For example if we have
1764
+	 * boundary="1" we want to ensure that we don't break out of every line
1765
+	 * that has -1 in it instead of starting --1. This needs some more work.
1751 1766
 	 */
1752
-	/*cli_dbgmsg("boundaryStart: line = '%s' boundary = '%s'\n", line, boundary);*/
1753
-	if(line == NULL)
1754
-		return 0;	/* empty line */
1755 1767
 	if(strstr(line, boundary) != NULL) {
1756 1768
 		cli_dbgmsg("found %s in %s\n", boundary, line);
1757 1769
 		return 1;
1758 1770
 	}
1759 1771
 	if(*line++ != '-')
1760 1772
 		return 0;
1761
-	if(*line++ != '-')
1762
-		return 0;
1763 1773
 	return strcasecmp(line, boundary) == 0;
1764 1774
 }
1765 1775