You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@bugzilla.spamassassin.org on 2012/09/28 23:34:04 UTC

[Bug 6845] New: MimeHeader plugin incorrectly detects MIME headers in message body

https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

          Priority: P2
            Bug ID: 6845
          Assignee: dev@spamassassin.apache.org
           Summary: MimeHeader plugin incorrectly detects MIME headers in
                    message body
          Severity: normal
    Classification: Unclassified
                OS: Linux
          Reporter: jhardin@impsec.org
          Hardware: PC
            Status: NEW
           Version: SVN Trunk (Latest Devel Version)
         Component: Plugins
           Product: Spamassassin

Created attachment 5093
  --> https://issues.apache.org/SpamAssassin/attachment.cgi?id=5093&action=edit
Malformed message, MIME headers in the body

Running a spample (attached) through my test bed I found a rule that should hit
this message (MIME_NO_TEXT) does not. Review of the debug log shows that the
MIMEHeader plugin is detecting the "Content-Type: text/plain" line in the
message body.

While these appear to be MIME headers, there is a blank line before them so
they are neither part of the message headers nor part of a MIME body part
header. They should not be detected by mimeheader rules.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

John Hardin <jh...@impsec.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #5094|0                           |1
        is obsolete|                            |

--- Comment #5 from John Hardin <jh...@impsec.org> ---
Created attachment 5095
  --> https://issues.apache.org/SpamAssassin/attachment.cgi?id=5095&action=edit
Neatened up initial patch

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

John Hardin <jh...@impsec.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #11 from John Hardin <jh...@impsec.org> ---
Commit 1397075

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

John Hardin <jh...@impsec.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|MimeHeader plugin           |Message parsing incorrectly
                   |incorrectly detects MIME    |detects MIME headers in
                   |headers in message body     |message body

--- Comment #2 from John Hardin <jh...@impsec.org> ---
The MIMEHeader plugin is a lightweight wrapper around the parsed message
structure. The problem is apparently somewhere in Message.pm

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

--- Comment #15 from Kevin A. McGrail <km...@pccc.com> ---
(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > spamassassin2.zones.apache.org svn updated so that this change will affect
> > > nightly masschecks
> > 
> > Doesn't that happen automatically?
> 
> Apparently not. When I ran "svn up" a *lot* of stuff was updated. Including
> the recent masscheck scripting changes.

Email me outside of bugzilla with what dir was it in because masscheck should
checkout the latest version from SVN as it runs?

FYI, my make test completed without issue as well.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

--- Comment #12 from John Hardin <jh...@impsec.org> ---
spamassassin2.zones.apache.org svn updated so that this change will affect
nightly masschecks

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

--- Comment #8 from Kevin A. McGrail <km...@pccc.com> ---
I would say any test to start with is good.  we can build on it from there. 
But I'm always afraid of these tiny core changes rippling things badly.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

John Hardin <jh...@impsec.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #5093|1                           |0
        is obsolete|                            |

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

--- Comment #10 from Kevin A. McGrail <km...@pccc.com> ---
(In reply to comment #9)
> Created attachment 5099 [details]
> patch with test
> 
> 
> (In reply to comment #8)
> > I would say any test to start with is good.  we can build on it from there. 
> > But I'm always afraid of these tiny core changes rippling things badly.
> 
> Okay, I took a look at the existing MIME parser tests. They appear to give
> fairly good coverage of valid forms and they all pass so I don't _think_ any
> new tests are needed there.
> 
> I did add one malformed MIME test that fails without the patch.

I like the test.  Applies clean to trunk, looks like it passes make test (have
a full running now but had other patches intermixed) and looks sound and I
think it should move forward.  Haven't tested my fears that it breaks something
internal though so some production esque testing would be nice from others.

I say commit it.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

--- Comment #3 from John Hardin <jh...@impsec.org> ---
Question about one possible approach to corrrecting this: would it be
reasonable to insert a blank line as the first line of the body lines array if
the MIME type is multipart, and the first line of the body lines array is not
blank, and the first line of the body lines array does not start with the MIME
boundary string? This seems to me to be a simple way to avoid the bug without
destabilizing the parser or making any substantive changes to the message being
scanned...

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

Kevin A. McGrail <km...@pccc.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kmcgrail@pccc.com

--- Comment #6 from Kevin A. McGrail <km...@pccc.com> ---
The patch seems straightforward but can you think of any way to add t/ tests to
check this code to make sure it doesn't have a larger impact than intended?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

John Hardin <jh...@impsec.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|Plugins                     |Libraries

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] MimeHeader plugin incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

John Hardin <jh...@impsec.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jhardin@impsec.org

--- Comment #1 from John Hardin <jh...@impsec.org> ---
Followup: if there are *two* blank lines between the message header and the
message body, the headers are properly ignored.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

John Hardin <jh...@impsec.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #5093|0                           |1
        is obsolete|                            |
   Attachment #5095|0                           |1
        is obsolete|                            |

--- Comment #9 from John Hardin <jh...@impsec.org> ---
Created attachment 5099
  --> https://issues.apache.org/SpamAssassin/attachment.cgi?id=5099&action=edit
patch with test


(In reply to comment #8)
> I would say any test to start with is good.  we can build on it from there. 
> But I'm always afraid of these tiny core changes rippling things badly.

Okay, I took a look at the existing MIME parser tests. They appear to give
fairly good coverage of valid forms and they all pass so I don't _think_ any
new tests are needed there.

I did add one malformed MIME test that fails without the patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

--- Comment #14 from John Hardin <jh...@impsec.org> ---
(In reply to comment #13)
> (In reply to comment #12)
> > spamassassin2.zones.apache.org svn updated so that this change will affect
> > nightly masschecks
> 
> Doesn't that happen automatically?

Apparently not. When I ran "svn up" a *lot* of stuff was updated. Including the
recent masscheck scripting changes.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

--- Comment #4 from John Hardin <jh...@impsec.org> ---
Created attachment 5094
  --> https://issues.apache.org/SpamAssassin/attachment.cgi?id=5094&action=edit
insert blank line in body lines array if needed to fix multipart parsing

Proposed patch per above suggestion, comments & votes solicited.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

--- Comment #13 from Kevin A. McGrail <km...@pccc.com> ---
(In reply to comment #12)
> spamassassin2.zones.apache.org svn updated so that this change will affect
> nightly masschecks

Doesn't that happen automatically?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 6845] Message parsing incorrectly detects MIME headers in message body

Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6845

--- Comment #7 from John Hardin <jh...@impsec.org> ---
(In reply to comment #6)
> The patch seems straightforward but can you think of any way to add t/ tests
> to check this code to make sure it doesn't have a larger impact than
> intended?

I'm not experienced in writing Perl test cases, but I'm sure it can be done. :)

I did run it through a make test and fixed one minor issue (I initially assumed
@message would never be empty); all existing tests pass.

A test case that expresses the problem (i.e. fails when the patch is not
present) is easy to design; figuring out what it might screw up is a little
more complicated. I went through my corpus looking for properly-formatted
multipart messages, and didn't see any possibilities where inserting a blank
line at the head this way would be an obvious problem.

I'll take a look at the existing tests and see if any of them can be easily
leveraged to add some cases that would indicate if this caused problems.
Suggestions solicited.

-- 
You are receiving this mail because:
You are the assignee for the bug.