Bug 2290 - Some material within base64-encoded mime boundaries ignored.
Summary: Some material within base64-encoded mime boundaries ignored.
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: spamassassin (show other bugs)
Version: SVN Trunk (Latest Devel Version)
Hardware: All All
: P5 normal
Target Milestone: 2.70
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on: 1527
Blocks:
  Show dependency tree
 
Reported: 2003-08-04 08:30 UTC by David Koppelman
Modified: 2004-01-27 09:50 UTC (History)
0 users


Attachment Type Modified Status Actions Submitter/CLA Status
Mail triggering bug. text/plain None David Koppelman [HasCLA]
Proposed fix. patch None David Koppelman [HasCLA]
Proposed fix. patch None David Koppelman [HasCLA]
Proposed fix. patch None David Koppelman [HasCLA]
Mail encountering bug fixed by patch 1218. text/plain None David Koppelman [HasCLA]
Show Obsolete (2) Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Koppelman 2003-08-04 08:30:05 UTC 
In the function get_decoded_stripped_body_text_array material within a
mime boundary is extracted for further processing.  The material is
not properly extracted when certain line endings are used.  This
occurs in the attached message.

The code also omits any line starting with "SPAM", perhaps this
code is obsolete.

I'll attach a patch fixing the two problems.
Comment 1 David Koppelman 2003-08-04 08:31:45 UTC 
Created attachment 1211 [details]
Mail triggering bug.
Comment 2 David Koppelman 2003-08-04 08:32:57 UTC 
Created attachment 1212 [details]
Proposed fix.

Patch was edited.
Comment 3 David Koppelman 2003-08-04 08:46:22 UTC 
Created attachment 1213 [details]
Proposed fix.

Minor change to proposed fix.
Comment 4 David Koppelman 2003-08-07 12:32:54 UTC 
I found a second bug which prevents some base64 material from
being used. (get_decoded_body_text_array discards
the blank line between the "Context-" lines and the base64-encoded
material.  A patch will be attached below.

I also just noticed that I didn't mention base64 encoding in the summary
or the previous comments.
Comment 5 David Koppelman 2003-08-07 12:33:42 UTC 
Created attachment 1218 [details]
Proposed fix.

Patch was editted.
Comment 6 David Koppelman 2003-08-07 12:38:43 UTC 
Created attachment 1219 [details]
Mail encountering bug fixed by patch 1218.

This message triggers the problem fixed by the last patch.
Comment 7 David Koppelman 2003-08-07 14:18:18 UTC 
I've been thinking...

Both bugs caused the code that's only supposed to remove the mime
boundaries and part headers to also remove message text.  (See the
first foreach loop in
PerMsgStatus::get_decoded_stripped_body_text_array.)  The patch that I
posted fixes the precise bugs that lead to this incorrect operation
(one caused by unexpected line endings, the other by an empty line
being discard) but not other potential problems that lead to
unexpected placement of whitespace and headers (causing the foreach
loop to remove too much text).

Given that the loop is susceptible to other problems and given that
the loop's only purpose is to remove stuff like:

    --DD381._.F.899C_
    Content-Type: text/html;
    Content-Transfer-Encoding: base64

I think the whole loop should just be eliminated or it should just
remove the boundary markers.  I doubt the conventional SA rules and BC
would be much effected by inclusion of the Content-Type text.  I don't
think the loop as it is should be retained with added safeguards such
as a counter on the number of lines past the boundary, because such
protections could possibly be exploited by those coveting the superior
qualities of SA users.
Comment 8 Theo Van Dinter 2004-01-27 18:50:37 UTC 
the new mime parser has no problem with these messages.