Bug #6023
closed
smtp: Attachment not being md5 matched
Added by Thomas Winter over 1 year ago.
Updated over 1 year ago.
Description
Previously we were using 4.0.6 and are in the process of upgrading due to EOL. Currently testing using 7.0.0-rc1.
EICAR file is 68 bytes and md5 is 44d88612fea8a8f36de82e1278abb02f but is not being matched.
Debug shows that the mime-decoder is using 70 bytes and including the \r\n delimiter, resulting in md5 of e7e5fa40569514ec442bbdf755d89c2f.
When retrieved via HTTP then the file is md5 matched.
I believe this to be caused by some of the recent mime and smtp fixes which included refactoring delimiter handling.
I have attached a small diff which makes the md5 get matched by excluding the delimiter when copying over to the buffer but I don't how valid this. This line was changed in commit b82b8825e79 (part of #5316 fix).
I've attached a packet capture of the smtp transaction, packet number 17 is the smtp packet with the attachment.
Files
I just noticed we had a patch to remove the \r\n that was being added to the end of lines so that md5 can be properly calculated. This piece of code was removed in commit b82b8825e79 but effectively still being done by including current_line_delimiter_len in the data to copy. Perhaps we can stop this as a config option?
It also made smtp process txt data in addition to attachments. Perhaps this can be applied?
This patch was applied to 3.0rc3 and carried through to 4.0.6.
hi @Thomas Winter, can you create a Suricata-Verify test and submit a PR for that? A suricata PR for the patch would also be great for testing and review. Thanks!
I have added a test based on the smtp packet capture used https://github.com/OISF/suricata-verify/pull/1202
The test currently fails because it expects to match on the EICAR md5 44d88612fea8a8f36de82e1278abb02f but the included delimiter causes the md5 to be calculated as e7e5fa40569514ec442bbdf755d89c2f.
If the changes in delim_diff.txt are applied then the test passes. However 3 other tests fail so this isn't valid fix, I've included these in verify_failures.txt.
- Assignee changed from OISF Dev to Philippe Antoine
- Target version changed from TBD to 7.0.0
- Status changed from New to In Review
- Status changed from In Review to Resolved
- Label Needs backport to 6.0 added
- Label deleted (
Needs backport to 6.0)
- Status changed from Resolved to Closed
Also available in: Atom
PDF