Banning .docm gives misleading error message

[Mark Martinec]

On 2016-04-05 10:21, Kai Risku wrote:
> We also have ClamAV blocking all files containing OLE2 Macros, so I am
> going for a belt-and-suspenders type of approach…
> 
> Regardless of the effectivity of blocking certain types of files or
> not, my main point for posting was the inability of amavis to clearly
> report which file was blocked when the blocked file is being detected
> as a container (e.g. a zip-file) containing other files.


[...]
> In order to guard against malicious macros, we have banned all
> macro-enabled Office document formats, i.e. added the following to
> $banned_filename_re:
> 
> # block macro-enabled office files
> qr'.\.(xlsm|xltm|xlam|docm|dotm|pptm|potm|ppam|ppsm|sldm)$'i,
> 
> Since modern Office documents are technically zip-files, amavisd-new
> opens and processes the zip archive. For originating (outgoing)
> messages we bounce the banned emails so the poor sender can understand
> why his emails are not delivered, but in this case amavisd-new does
> not report the actual office document being banned but instead blames
> the first file inside the zip-archive. This results in very cryptic
> error messages, like:
> 
>                              Subject: BANNED contents from you
> (.txt,[Content_Types].xml)
> 
> Our content checker found
>     banned name: .txt,[Content_Types].xml
>                              in email presumable from you
> 
> It seems amavisd has a small “optimization” that skips banned checks
> for non-leaf nodes. I propose removing that so the actual office
> documents can be directly banned and correctly reported:
> 
> @@ -9912,7 +9912,9 @@
>        my(@path) = @{$part->path};
>        next  if @path <= 1;
>        shift(@path);  # ignore place-holder root node
> -      next  if @{$part->children};  # ignore non-leaf nodes
> +      # also process non-leaf nodes or we cannot block office 
> documents
> +      # without alert about wrong parts (blames the innocent zip 
> member)
> +      # next  if @{$part->children};  # ignore non-leaf nodes
>        my(@descr_trad);  # a part path: list of predecessors of a 
> message part
>        my(@descr);  # same, but in form suitable for check on 
> banned_namepath_re
>        for my $p (@path) {



I don't think the solution is not to ignore non-leaf nodes.
The non-leaf nodes information will be visible and checked
during processing of each leaf node - as their parent nodes.
This provides more information to checker, putting each
leaf node in its context.

The issue here is reporting, not checking. The %F macro
expands to whatever a method banning_reason_short() povides.
Perhaps this method should enhanced, or the notification
template should use more informative macros instead of %F
(in $notify_sender_templ ) :

   BANNED contents from you (%F)


The following macros are available (README.customize) :
(or a new macro could be devised)

   banning_rule_key  a lookup key (a regexp) of a banning rule which 
matched,
                 e.g.:  (?-xism:^\\.(exe-ms|dll)$(?# rule #9))
   banning_rule_comment  a comment from a regexp in banning_rule_key, or
                 the whole banning_rule_key if it does not contain a 
comment,
                 e.g.:  rule #9
   banning_rule_rhs  right-hand side of a banning rule which matched, 
often
                 just a '1', but can be any string which evaluates to 
true
   banned_parts  a list of banned parts, with their full path in a 
MIME/archive
      e.g.:  multipart/mixed | 
application/octet-stream,.exe,.exe-ms,videos.exe

   banned_parts_as_attr  similar to banned_parts, but uses a different 
syntax
                 using attribute/value pairs; currently known attributes 
are:
      P: part's base name, i.e. a file name in a ./parts/ temporary 
directory
      L: part's location (path) in a mail tree
      M: MIME type as declared in MIME header fields of a message
      T: short part's content type according to a file(1) utility
      N: declared part names (none, one or more), as declared in MIME 
header
         fields or in an archive (tar, zip, ...)
      A: part's attributes as follows:
         U=undecodable, C=crypted, D=directory, S=special(device), L=link
      e.g.:
        P=p003,L=1,M=multipart/mixed |
        P=p002,L=1/2,M=application/octet-stream,T=rar,N=Setup1.1.rar |
        P=p007,L=1/2/4,T=exe,T=exe-ms,N=setup.exe

   F  just the first leaf node from banned_parts, with a prepended rule 
comment
      (if any), e.g.:  rule 
#9:application/octet-stream,.exe,.exe-ms,videos.exe


Mark