Mailscanner converting HTML messages with FORM tags

[Lancaster, David Matthew]

Hi Julian!

I think we all understand that there never seems to be enough hours in the day.

Anyways,  I think there's a bit of confusion going on here.
I don't need the ability to perform "snazzy" filtering of the HTML (least not
yet), and I don't think anybody else has mentioned it yet (although it might
make a nice future enhancement).

I just want to be able to set the following all at once:
1. Allow Object Codebase messages, but Convert them to text
2. Allow Iframes messages, but Convert them to text
3. Allow Form messages to pass without modification.
which I can't because "Convert Dangerous HTML" is a "clobber-all" or nothing...

This is what I've come up with so far.  Patch changes:
- Allow Iframe, Allow Form, Allow Object Codebase now have three options {no |
yes | convert}
- The "Convert Dangerous HTML" feature still functions (for backwards
compatibility) and will convert *anything* set to "yes" or "convert" above.

To continue with the old settings, do nothing.
To use the new settings, set Convert Dangerous HTML to "no" and use the
distinct
options.
("Convert Dangerous HTML" could be deprecated in a future major version change
if this change is considered useful).

The code seems to work, at least on my test box.   Works with a small ruleset
too...
It could probably use some neatening (getting rid of the embedded constants,
etc), but the functionality is all there.
If this modification is worth carrying forward, I could tidy it up (or Julian
could if he wanted cleaner code ;) )

D.


P.S.  If the HTML-tag-only-cleaning feature was added, it should be simple add a
new option to each of the Allow directives, e.g. {no|yes|convert|conv_tags},
etc.  But we can leave that for another day...



> Hi guys!
>
> Sorry haven't been around much recently, have had a lot of other things on.
>
> I like the idea of the
> Convert Dangerous HTML = yes | no | object-codebase | iframe | form
> where there can be more than 1 option give in that line. The only snag
> being you don't know which tags cause the entire HTML to be removed, and
> which tags cause just those tags to be removed.
>
> So maybe your solution is better. What's the difference in behaviour
> between "no" and "convert-all"?
>
> The awkward bit is implementing it, it's all to do with HTML::TokeParser
> and related things.
> The code that currently does the job is in Message.pm. You want "sub
> HTMLEntityToText" and the "sub get_text" following it (which over-rides the
> TokeParser's original code so it has a slightly different output).
>
> All it can do now is strip all HTML tags leaving the plain text.
>
> As a trial for what I am going to put into the main code, try running the
> attached script, passing an HTML file on the command line.
>
-------------- next part --------------
diff -ru ../MailScanner.orig/ConfigDefs.pl ./ConfigDefs.pl
--- ../MailScanner.orig/ConfigDefs.pl	2003-10-06 05:03:16.000000000 -0300
+++ ./ConfigDefs.pl	2003-10-27 15:59:02.000000000 -0400
@@ -361,9 +361,9 @@
 VirusSubjectText		{VIRUS?}
 
 [All,YesNo]
-AllowIFrameTags		0	no	0	yes	1
-AllowFormTags		0	no	0	yes	1
-AllowObjectTags		0	no	0	yes	1
+AllowIFrameTags		0	no	0	convert	1	yes	2
+AllowFormTags		0	no	0	convert	1	yes	2
+AllowObjectTags		0	no	0	convert	1	yes	2
 BlockEncrypted		0	no	0	yes	1
 BlockUnencrypted	0	no	0	yes	1
 DeliverCleanedMessages	1	no	0	yes	1
diff -ru ../MailScanner.orig/SweepContent.pm ./SweepContent.pm
--- ../MailScanner.orig/SweepContent.pm	2003-10-06 05:03:16.000000000 -0300
+++ ./SweepContent.pm	2003-10-27 16:03:11.000000000 -0400
@@ -113,21 +113,25 @@
       $counter++;
     }
 
-    # Search for Microsoft-specific attacks
-    # Disallow both by default. Allow them only if all addresses agree.
+    # Search for HTML-specific attacks
+    # Disallow by default. 
     $allowiframes = 0;
     $allowobjects = 0;
     $allowforms   = 0;
-    $allowiframes = 1
-      if MailScanner::Config::Value('allowiframetags', $message) =~ /^[1\s]+$/;
-    $allowobjects = 1
-      if MailScanner::Config::Value('allowobjecttags', $message) =~ /^[1\s]+$/;
-    $allowforms   = 1
-      if MailScanner::Config::Value('allowformtags', $message) =~ /^[1\s]+$/;
+    #Higher security options override lower security options, for rulesets
+    $allowiframes = 2 if MailScanner::Config::Value('allowiframetags', $message) =~ /2/;
+    $allowiframes = 1 if MailScanner::Config::Value('allowiframetags', $message) =~ /1/;
+    $allowiframes = 0 if MailScanner::Config::Value('allowiframetags', $message) =~ /0/;
+    $allowobjects = 2 if MailScanner::Config::Value('allowobjecttags', $message) =~ /2/;
+    $allowobjects = 1 if MailScanner::Config::Value('allowobjecttags', $message) =~ /1/;
+    $allowobjects = 0 if MailScanner::Config::Value('allowobjecttags', $message) =~ /0/;
+    $allowforms   = 2 if MailScanner::Config::Value('allowformtags', $message) =~ /2/;
+    $allowforms   = 1 if MailScanner::Config::Value('allowformtags', $message) =~ /1/;
+    $allowforms   = 0 if MailScanner::Config::Value('allowformtags', $message) =~ /0/;
     $stripdangerous = MailScanner::Config::Value('stripdangeroustags',$message);
     # Shortcut the check completely if they want to allow everything
     # and are not converting nasty tags to text
-    if (!($allowiframes && $allowforms && $allowobjects && !$stripdangerous) &&
+    if (!(($allowiframes==2) && ($allowforms==2) && ($allowobjects==2) && !$stripdangerous) &&
         FindHTMLExploits($message, $id, $ent, $allowiframes,
                          $allowobjects, $allowforms, $stripdangerous)) {
       $counter++;
@@ -341,7 +345,7 @@
         if MailScanner::Config::Value('logiframetags', $message);
       # Mark the message
       if ($allowiframes) {
-        if ($stripdangerous) {
+        if ($stripdangerous or ($allowiframes==1)) {
           $message->{needsstripping} = 1;
           $message->{bodymodified}   = 1; # Mark it for rebuilding
           $counter++;
@@ -362,7 +366,7 @@
       #  if MailScanner::Config::Value('logformtags', $message);
       # Mark the message
       if ($allowforms) {
-        if ($stripdangerous) {
+        if ($stripdangerous or ($allowforms==1)) {
           $message->{needsstripping} = 1;
           $message->{bodymodified}   = 1; # Mark it for rebuilding
           $counter++;
@@ -378,7 +382,7 @@
     }
     if ($codebasefound) {
       if ($allowobjects) {
-        if ($stripdangerous) {
+        if ($stripdangerous or ($allowobjects==1)) {
           $message->{needsstripping} = 1;
           $message->{bodymodified}   = 1; # Mark it for rebuilding
           $counter++;