SA Bugzilla – Bug 6823
[review] Malformed messages allow evasion of URIBL checks / Last body line skipped if multipart end boundary missing
Last modified: 2012-09-27 07:05:25 UTC
A malformed MIME message with an URI in the last line of the full body allows perfectly evading URIBL checks, because the URI is totally ignored. This has been used by a spamming operation for months with massive spam-runs. See the attached sample for details.
Created attachment 5083 [details] A malformed message which allows evading URI checks.
Just to be clear: URIBL checks fail because the last line of these messages is not considered part of the body, so uri and rawbody rules do fail as well. A full rule is needed to be able to match the line content.
It seems Message.pm does not handle missing boundary end well. From what I see, it has been like this since 2004.. # if we're on the last body line, or we find any boundary marker, # deal with the mime part if ( --$line_count == 0 || (defined $boundary && /^--\Q$boundary\E(?:--)?\s*$/) ) { my $line = $_; # remember the last line After that, there's nothing adding the "remembered $line" to body array, so indeed it goes missing. I could come up with a quick patch, not sure if there are any caveeats..
Created attachment 5085 [details] Proposed fix Here's a minimum change patch proposal. I've run it against 20000s/10000h multipart messages and reviewed the diffs of internal data (Data::Dumper(find_parts)) by hand. Zero differences in ham and only few dozen cases of missing end boundary in spam. But that's my very old corpus. So the change looks safe to me.. waiting for another opinion to commit. ;-)
+1 from me. I would say this is urgent and needs testing but from your code, you are just pushing the last part of the message into the buffer and I can't predict many issues. Great catch. And definitely open another ticket for the idea " Could "missing end boundary" be a useful rule?" I don't think the concern that the message was modified/corrupted along the way should be consider valid. It will fail many other checks such as DKIM as well. regards, KAM
Ok patch as is in trunk.. Sending Message.pm Transmitting file data . Committed revision 1372304.
> Ok patch as is in trunk.. > Committed revision 1372304. +1 looks alright to me (code inspection)
I guess it works so resolved.