Security #6279
closedCrash in SMTP parser during parsing of email
Description
During testing of Suricata 7.0, we've noticed that Suricata occationally exits due to issues with SMTP traffic.
- SCMd5Update+0x00000013
- MimeDecParseLine+0x00000101
- SMTPProcessRequest.isra.15+0x000004ba
- SMTPPreProcessCommands.isra.16+0x0000010b
- SMTPParse+0x00000157
- AppLayerParserParse+0x00000343
The bug has been reproduced on suricata-7.0.0 (21ec99aa7).
Quick testing with SMTP applayer set to detection only and the file logger being configured to force MD5 hashing suggests the issue isn't tied directly to the new Rust MD5 hashing.
We have acquired a copy of the traffic that triggers the flow, and produced a minimal PCAP for reproducing this.
Removing either the Received header or the PIPELINING feature flag stops the crash.
Files
Updated by Shivani Bhardwaj over 1 year ago
- Status changed from New to Assigned
- Assignee changed from OISF Dev to Shivani Bhardwaj
Updated by Jason Ish over 1 year ago
- Tracker changed from Bug to Security
- Private changed from No to Yes
- Severity set to MODERATE
Updated by Simen Lybekk over 1 year ago
- File poc-security-6279.pcap poc-security-6279.pcap added
- Description updated (diff)
Updated by Jason Ish over 1 year ago
I am able to reproduce, but not in the context of any filestore configuration. Instead by enabling the body-md5
features of the SMTP parser.
Updated by Simen Lybekk over 1 year ago
Jason Ish wrote in #note-4:
I am able to reproduce, but not in the context of any filestore configuration. Instead by enabling the
body-md5
features of the SMTP parser.
That is correct.
I was trying to say that it's not happening due to MD5 hashing in general, and that this is the only code path we've gotten the crash for.
The comment was from before we had a reproduction PCAP and associated core dump, just the libunwind callstack.
Updated by Jason Ish over 1 year ago
So quick analysis.. At util-decode-mime.c:2355, SCMd5Update
is being called with a NULL md5 context. SCMd5Update
has the expectation that its self
object is valid.
So while this is a state-machine type error in the MIME decoding, I'm thinking some assertions on the Rust side are also in order. Suricata will still crash, but it'll be absolutely clear that the contract between the function and the caller wasn't honored.
Updated by Philippe Antoine over 1 year ago
NULL deference stack trace is
AddressSanitizer:DEADLYSIGNAL ================================================================= ==32122==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000058 (pc 0x000109e933a8 bp 0x70000f4ef9f0 sp 0x70000f4ef9c0 T13) ==32122==The signal is caused by a READ memory access. ==32122==Hint: address points to the zero page. #0 0x109e933a8 in SCMd5Update hashing.rs:144 #1 0x109c5d75a in MimeDecParseLine util-decode-mime.c:2569 #2 0x1098fe22b in SMTPProcessRequest app-layer-smtp.c:1299 #3 0x1098fc22f in SMTPPreProcessCommands app-layer-smtp.c:1393 #4 0x1098fabc0 in SMTPParse app-layer-smtp.c:1465 #5 0x1098f6d22 in SMTPParseClientRecord app-layer-smtp.c:1506 #6 0x1098f087c in AppLayerParserParse app-layer-parser.c:1407 #7 0x10982c039 in AppLayerHandleTCPData app-layer.c:773 #8 0x109c13d06 in StreamTcpReassembleAppLayer stream-tcp-reassemble.c:1391 #9 0x109c18804 in StreamTcpReassembleHandleSegment stream-tcp-reassemble.c:1997 #10 0x109bc65d3 in StreamTcpStateDispatch stream-tcp.c:5236 #11 0x109bb4eaf in StreamTcpPacket stream-tcp.c:5433 #12 0x109be7a97 in StreamTcp stream-tcp.c:5745 #13 0x109b0636e in FlowWorkerStreamTCPUpdate flow-worker.c:391 #14 0x109b03ff5 in FlowWorker flow-worker.c:607 #15 0x109c38990 in TmThreadsSlotVar tm-threads.c:469
Updated by Philippe Antoine over 1 year ago
- Status changed from Assigned to In Review
- Assignee changed from Shivani Bhardwaj to Philippe Antoine
- Target version changed from TBD to 7.0.1
- Label Needs backport, Needs backport to 6.0 added
Gitlab MR
Updated by OISF Ticketbot over 1 year ago
- Label deleted (
Needs backport, Needs backport to 6.0)
Updated by Simen Lybekk over 1 year ago
Updated by Philippe Antoine over 1 year ago
- Severity changed from MODERATE to HIGH
I think this is a feature "disabled by default Tier 1", so HIGH severity
Updated by Victor Julien over 1 year ago
- Status changed from In Review to Resolved
Updated by Victor Julien over 1 year ago
- Status changed from Resolved to Closed
Updated by Philippe Antoine over 1 year ago
- Related to Optimization #3591: fuzz: target with pcap, rules and yaml added