Actions
Bug #3820
closedssh: invalid use to 'AppLayerResult::incomplete`
Added by Victor Julien over 4 years ago. Updated over 4 years ago.
Affected Versions:
Effort:
Difficulty:
Label:
Description
[1920] 9/7/2020 -- 06:56:24 - (suricata.c:2297) <Notice> (PostRunStartedDetectSetup) -- Signature(s) loaded, Detect thread(s) activated.
suricata: app-layer-parser.c:1257: AppLayerParserParse: Assertion `!(res.needed + res.consumed < input_len)' failed.
Aborted (core dumped)
(gdb) f 4
#4 0x000055c2e8fcb49e in AppLayerParserParse (tv=0x6120000a9840, alp_tctx=0x6190000af080, f=0x61300098c1c0, alproto=5, flags=8 '\b', input=0x61d0015fb8a7 "", input_len=988) at app-layer-parser.c:1257
1257 BUG_ON(res.needed + res.consumed < input_len);
(gdb) p res
$1 = {status = 1, consumed = 6, needed = 84}
2nd time I've seen this, 1st time I was able to catch it. This started happening after the hassh code was merged.
Files
| ssh-s101.pcap (13.8 KB) ssh-s101.pcap | Victor Julien, 07/10/2020 05:38 AM |
Updated by Victor Julien over 4 years ago
[21898] 9/7/2020 -- 16:06:19 - (ssh.rs:234) <Notice> (<rust>) -- r AppLayerResult { status: 1, consumed: 6, needed: 528 }
Updated by Victor Julien over 4 years ago
parser::MessageCode::SshMsgKexinit if hassh_is_enabled() => {
// check if buffer is bigger than maximum reassembled packet size
if hdr.record_left < SSH_MAX_REASSEMBLED_RECORD_LEN as u32 {
// saving type of incomplete kex message
hdr.record_left_msg = parser::MessageCode::SshMsgKexinit;
let r = AppLayerResult::incomplete(
SSH_RECORD_HEADER_LEN as u32,
hdr.record_left as u32
);
SCLogNotice!("r {:?}", r);
return r;
}
else {
SCLogDebug!("SSH buffer is bigger than maximum reassembled packet size");
self.set_event(SSHEvent::LongKexRecord);
}
}
The error seems to be in this block.
Updated by Vadym Malakhatko over 4 years ago
Yes, I'm testing. Would be great if you'll upload pcap.
Updated by Victor Julien over 4 years ago
- File ssh-s101.pcap ssh-s101.pcap added
Default yaml with only hassh enabled:
Starting program: /home/victor/sync/devel/suricata-rust/src/suricata -c suricata.yaml -l tmp/ -r ssh-s101.pcap --runmode=single --disable-detection --simulate-ips
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[28076] 10/7/2020 -- 07:40:07 - (suricata.c:1342) <Info> (ParseCommandLine) -- Setting IPS mode
[28076] 10/7/2020 -- 07:40:07 - (suricata.c:1065) <Notice> (LogVersion) -- This is Suricata version 6.0.0-dev running in USER mode
[New Thread 0x7fffeead4700 (LWP 28080)]
[New Thread 0x7fffeddd5700 (LWP 28081)]
[New Thread 0x7fffed5d4700 (LWP 28082)]
[New Thread 0x7fffecdd3700 (LWP 28083)]
[New Thread 0x7fffec5d2700 (LWP 28084)]
[28076] 10/7/2020 -- 07:40:07 - (tm-threads.c:1888) <Notice> (TmThreadWaitOnThreadInit) -- all 1 packet processing threads, 4 management threads initialized, engine started.
suricata: app-layer-parser.c:1256: int AppLayerParserParse(ThreadVars *, AppLayerParserThreadCtx *, Flow *, AppProto, uint8_t, const uint8_t *, uint32_t): Assertion `!(res.needed + res.consumed < input_len)' failed.
Thread 2 "W#01" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fffeead4700 (LWP 28080)]
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ffff39048b1 in __GI_abort () at abort.c:79
#2 0x00007ffff38f442a in __assert_fail_base (fmt=0x7ffff3a7ba38 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x1f3f680 <.str.56> "!(res.needed + res.consumed < input_len)", file=file@entry=0x1f3d500 <.str> "app-layer-parser.c",
line=line@entry=1256, function=function@entry=0x1f3f560 <__PRETTY_FUNCTION__.AppLayerParserParse> "int AppLayerParserParse(ThreadVars *, AppLayerParserThreadCtx *, Flow *, AppProto, uint8_t, const uint8_t *, uint32_t)") at assert.c:92
#3 0x00007ffff38f44a2 in __GI___assert_fail (assertion=0x1f3f680 <.str.56> "!(res.needed + res.consumed < input_len)", file=0x1f3d500 <.str> "app-layer-parser.c", line=1256,
function=0x1f3f560 <__PRETTY_FUNCTION__.AppLayerParserParse> "int AppLayerParserParse(ThreadVars *, AppLayerParserThreadCtx *, Flow *, AppProto, uint8_t, const uint8_t *, uint32_t)") at assert.c:101
#4 0x00000000006f010c in AppLayerParserParse (tv=0x6120000f9ac0, alp_tctx=0x6190002c0680, f=0x6130001216c0, alproto=5, flags=4 '\004', input=0x61d000384a80 "", input_len=1440) at app-layer-parser.c:1256
#5 0x00000000004e6dbc in AppLayerHandleTCPData (tv=0x6120000f9ac0, ra_ctx=0x6030002e79e0, p=0x61e0003fb480, f=0x6130001216c0, ssn=0x6120001bbec0, stream=0x7fffeeacf440, data=0x61d000384a80 "", data_len=1440, flags=4 '\004') at app-layer.c:670
#6 0x0000000001596b54 in ReassembleUpdateAppLayer (tv=0x6120000f9ac0, ra_ctx=0x6030002e79e0, ssn=0x6120001bbec0, stream=0x7fffeeacf440, p=0x61e0003fb480, dir=UPDATE_DIR_PACKET) at stream-tcp-reassemble.c:1098
#7 0x0000000001594699 in StreamTcpReassembleAppLayer (tv=0x6120000f9ac0, ra_ctx=0x6030002e79e0, ssn=0x6120001bbec0, stream=0x6120001bbf58, p=0x61e0003fb480, dir=UPDATE_DIR_PACKET) at stream-tcp-reassemble.c:1155
#8 0x000000000159f35d in StreamTcpReassembleHandleSegment (tv=0x6120000f9ac0, ra_ctx=0x6030002e79e0, ssn=0x6120001bbec0, stream=0x6120001bbf58, p=0x61e0003fb480, pq=0x6060002230a8) at stream-tcp-reassemble.c:1820
#9 0x000000000152ab59 in HandleEstablishedPacketToServer (tv=0x6120000f9ac0, ssn=0x6120001bbec0, p=0x61e0003fb480, stt=0x6060002230a0, pq=0x6060002230a8) at stream-tcp.c:2294
#10 0x00000000014b99a8 in StreamTcpPacketStateEstablished (tv=0x6120000f9ac0, p=0x61e0003fb480, stt=0x6060002230a0, ssn=0x6120001bbec0, pq=0x6060002230a8) at stream-tcp.c:2668
#11 0x0000000001439949 in StreamTcpStateDispatch (tv=0x6120000f9ac0, p=0x61e0003fb480, stt=0x6060002230a0, ssn=0x6120001bbec0, pq=0x6060002230a8, state=4 '\004') at stream-tcp.c:4687
#12 0x000000000142cd7c in StreamTcpPacket (tv=0x6120000f9ac0, p=0x61e0003fb480, stt=0x6060002230a0, pq=0x606000223068) at stream-tcp.c:4876
#13 0x000000000143b079 in StreamTcp (tv=0x6120000f9ac0, p=0x61e0003fb480, data=0x6060002230a0, pq=0x606000223068) at stream-tcp.c:5212
#14 0x000000000129e02a in FlowWorker (tv=0x6120000f9ac0, p=0x61e0003fb480, data=0x606000223040) at flow-worker.c:240
#15 0x000000000162f3c2 in TmThreadsSlotVarRun (tv=0x6120000f9ac0, p=0x61e0003fb480, slot=0x606000222a40) at tm-threads.c:117
#16 0x0000000001425ad5 in TmThreadsSlotProcessPkt (tv=0x6120000f9ac0, s=0x606000222a40, p=0x61e0003fb480) at ./tm-threads.h:192
#17 0x000000000142437c in PcapFileCallbackLoop (user=0x60700013a7d0 "\020\340@", h=0x7fffeead3300, pkt=0x7fffee293800 "\220\342\272\001|9\350\337p\243f\222\b") at source-pcap-file-helper.c:101
#18 0x00007ffff6955dd9 in ?? () from /usr/lib/x86_64-linux-gnu/libpcap.so.0.8
#19 0x0000000001422a5b in PcapFileDispatch (ptv=0x60700013a7d0) at source-pcap-file-helper.c:146
#20 0x000000000141ab65 in ReceivePcapFileLoop (tv=0x6120000f9ac0, data=0x60b00006b020, slot=0x6060002229e0) at source-pcap-file.c:176
#21 0x000000000163c81d in TmThreadsSlotPktAcqLoop (td=0x6120000f9ac0) at tm-threads.c:300
#22 0x00007ffff5ee96db in start_thread (arg=0x7fffeead4700) at pthread_create.c:463
#23 0x00007ffff39e5a3f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
(gdb) f 4
#4 0x00000000006f010c in AppLayerParserParse (tv=0x6120000f9ac0, alp_tctx=0x6190002c0680, f=0x6130001216c0, alproto=5, flags=4 '\004', input=0x61d000384a80 "", input_len=1440) at app-layer-parser.c:1256
1256 BUG_ON(res.needed + res.consumed < input_len);
(gdb) p res
$1 = {status = 1, consumed = 6, needed = 528}
(gdb)
The
--simulate-ips option is critical here: w/o it the condition won't trigger. My assumption is that this option is more aggressive at sending incomplete data to the parser.Updated by Philippe Antoine over 4 years ago
- Status changed from Assigned to In Review
Updated by Philippe Antoine over 4 years ago
- Status changed from In Review to Closed
Updated by Philippe Antoine over 4 years ago
- Status changed from Closed to In Review
First fix seemed incomplete even if it was enough for this attached pcap
https://github.com/OISF/suricata/pull/5177
https://github.com/OISF/suricata-verify/pull/273
Updated by Victor Julien over 4 years ago
- Status changed from In Review to Closed
- Assignee changed from Vadym Malakhatko to Philippe Antoine
- Priority changed from High to Normal
Actions