Bug #6023
closedsmtp: Attachment not being md5 matched
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
Updated by Thomas Winter over 1 year ago
- File old_smtp_diff.txt old_smtp_diff.txt added
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.
Updated by Victor Julien over 1 year ago
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!
Updated by Thomas Winter over 1 year ago
- File verify_failures.txt verify_failures.txt added
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.
Updated by Philippe Antoine over 1 year ago
- Assignee changed from OISF Dev to Philippe Antoine
Updated by Philippe Antoine over 1 year ago
- Target version changed from TBD to 7.0.0
Updated by Philippe Antoine over 1 year ago
- Status changed from New to In Review
Updated by Victor Julien over 1 year ago
- Status changed from In Review to Resolved
- Label Needs backport to 6.0 added
Updated by Victor Julien over 1 year ago
- Status changed from Resolved to Closed