cisagov / icsnpp-bacnet

Zeek BACnet Parser - CISA ICSNPP
BSD 3-Clause "New" or "Revised" License
15 stars 11 forks source link

Segfaults on live data #14

Closed keithjjones closed 1 year ago

keithjjones commented 1 year ago

🐛 Summary

I am running this analyzer on a large university network that also has a lot of connection gaps. I saw this segfault (it's happened multiple times), but I don't know what traffic this happens on and I wouldn't be able to share the traffic from their network if I could figure out what caused it. Is this trace something that rings a bell from the code stored here? Thanks!

2022-09-15T18:42:40.077304134Z worker-29: stacktrace: _ZN6binpac6BACNET11BACNET_Flow12process_i_amEPSt6vectorIPNS0_10BACnet_TagESaIS4_EE;_ZN6binpac6BACNET23Unconfirmed_Request_PDU5ParseEPKhS3_PNS0_13ContextBACNETEi;_ZN6binpac6BACNET11APDU_Header5ParseEPKhS3_PNS0_13ContextBACNETEi;_ZN6binpac6BACNET11NPDU_Header5ParseEPKhS3_PNS0_13ContextBACNETEi;_ZN6binpac6BACNET14Forwarded_NPDU5P
arseEPKhS3_PNS0_13ContextBACNETEi;_ZN6binpac6BACNET11BVLC_Header5ParseEPKhS3_PNS0_13ContextBACNETEi;_ZN6binpac6BACNET11BACNET_Flow7NewDataEPKhS3_;_ZN8analyzer6BACNET15BACNET_Analyzer13DeliverPacketEiPKhbmPKN4zeek6IP_HdrEi;_ZN4zeek8analyzer8Analyzer10NextPacketEiPKhbmPKNS_6IP_HdrEi;_ZN4zeek8analyzer8Analyzer13ForwardPacketEiPKhbmPKNS_6IP_HdrEi;_ZN4zeek15packet_analysis3UDP11UDPAna
lyzer13DeliverPacketEPNS_10ConnectionEdbiPNS_6PacketE;_ZN4zeek15packet_analysis2IP15IPBasedAnalyzer13AnalyzePacketEmPKhPNS_6PacketE;_ZNK4zeek15packet_analysis8Analyzer13ForwardPacketEmPKhPNS_6PacketEj;_ZN4zeek15packet_analysis2IP10IPAnalyzer13AnalyzePacketEmPKhPNS_6PacketE;_ZNK4zeek15packet_analysis8Analyzer13ForwardPacketEmPKhPNS_6PacketEj;_ZNK4zeek15packet_analysis8Analyzer13Fo
rwardPacketEmPKhPNS_6PacketEj;_ZNK4zeek15packet_analysis8Analyzer13ForwardPacketEmPKhPNS_6PacketEj;_ZNK4zeek15packet_analysis8Analyzer13ForwardPacketEmPKhPNS_6PacketEj;_ZN4zeek15packet_analysis7Manager13ProcessPacketEPNS_6PacketE;_ZN4zeek9run_state6detail15dispatch_packetEPNS_6PacketEPNS_8iosource6PktSrcE;_ZN4zeek8iosource6PktSrc7ProcessEv;_ZN4zeek9run_state6detail8run_loopEv;mai
n;__libc_start_main;_start
2022-09-15T18:42:41.000971596Z worker-29: Segmentation fault
2022-09-15T18:42:41.001109703Z worker-29 failed. Restarting.
keithjjones commented 1 year ago

I was able to demangle the names:

$ cat t | c++filt
binpac::BACNET::BACNET_Flow::process_i_am(std::vector<binpac::BACNET::BACnet_Tag*, std::allocator<binpac::BACNET::BACnet_Tag*> >*);binpac::BACNET::Unconfirmed_Request_PDU::Parse(unsigned char const*, unsigned char const*, binpac::BACNET::ContextBACNET*, int);binpac::BACNET::APDU_Header::Parse(unsigned char const*, unsigned char const*, binpac::BACNET::ContextBACNET*, int);binpac:
:BACNET::NPDU_Header::Parse(unsigned char const*, unsigned char const*, binpac::BACNET::ContextBACNET*, int);_ZN6binpac6BACNET14Forwarded_NPDU5P
arseEPKhS3_PNS0_13ContextBACNETEi;binpac::BACNET::BVLC_Header::Parse(unsigned char const*, unsigned char const*, binpac::BACNET::ContextBACNET*, int);binpac::BACNET::BACNET_Flow::NewData(unsigned char const*, unsigned char const*);analyzer::BACNET::BACNET_Analyzer::DeliverPacket(int, unsigned char const*, bool, unsigned long, zeek::IP_Hdr const*, int);zeek::analyzer::Analyzer::Ne
xtPacket(int, unsigned char const*, bool, unsigned long, zeek::IP_Hdr const*, int);zeek::analyzer::Analyzer::ForwardPacket(int, unsigned char const*, bool, unsigned long, zeek::IP_Hdr const*, int);_ZN4zeek15packet_analysis3UDP11UDPAna
lyzer13DeliverPacketEPNS_10ConnectionEdbiPNS_6PacketE;zeek::packet_analysis::IP::IPBasedAnalyzer::AnalyzePacket(unsigned long, unsigned char const*, zeek::Packet*);zeek::packet_analysis::Analyzer::ForwardPacket(unsigned long, unsigned char const*, zeek::Packet*, unsigned int) const;zeek::packet_analysis::IP::IPAnalyzer::AnalyzePacket(unsigned long, unsigned char const*, zeek::Pac
ket*);zeek::packet_analysis::Analyzer::ForwardPacket(unsigned long, unsigned char const*, zeek::Packet*, unsigned int) const;_ZNK4zeek15packet_analysis8Analyzer13Fo
rwardPacketEmPKhPNS_6PacketEj;zeek::packet_analysis::Analyzer::ForwardPacket(unsigned long, unsigned char const*, zeek::Packet*, unsigned int) const;zeek::packet_analysis::Analyzer::ForwardPacket(unsigned long, unsigned char const*, zeek::Packet*, unsigned int) const;zeek::packet_analysis::Manager::ProcessPacket(zeek::Packet*);zeek::run_state::detail::dispatch_packet(zeek::Packet
*, zeek::iosource::PktSrc*);zeek::iosource::PktSrc::Process();zeek::run_state::detail::run_loop();mai
n;__libc_start_main;_start
keithjjones commented 1 year ago

With newlines:

$ cat t | c++filt | tr ';' '\n'
binpac::BACNET::BACNET_Flow::process_i_am(std::vector<binpac::BACNET::BACnet_Tag*, std::allocator<binpac::BACNET::BACnet_Tag*> >*)
binpac::BACNET::Unconfirmed_Request_PDU::Parse(unsigned char const*, unsigned char const*, binpac::BACNET::ContextBACNET*, int)
binpac::BACNET::APDU_Header::Parse(unsigned char const*, unsigned char const*, binpac::BACNET::ContextBACNET*, int)
binpac::BACNET::NPDU_Header::Parse(unsigned char const*, unsigned char const*, binpac::BACNET::ContextBACNET*, int)
_ZN6binpac6BACNET14Forwarded_NPDU5P
arseEPKhS3_PNS0_13ContextBACNETEi
binpac::BACNET::BVLC_Header::Parse(unsigned char const*, unsigned char const*, binpac::BACNET::ContextBACNET*, int)
binpac::BACNET::BACNET_Flow::NewData(unsigned char const*, unsigned char const*)
analyzer::BACNET::BACNET_Analyzer::DeliverPacket(int, unsigned char const*, bool, unsigned long, zeek::IP_Hdr const*, int)
zeek::analyzer::Analyzer::NextPacket(int, unsigned char const*, bool, unsigned long, zeek::IP_Hdr const*, int)
zeek::analyzer::Analyzer::ForwardPacket(int, unsigned char const*, bool, unsigned long, zeek::IP_Hdr const*, int)
_ZN4zeek15packet_analysis3UDP11UDPAna
lyzer13DeliverPacketEPNS_10ConnectionEdbiPNS_6PacketE
zeek::packet_analysis::IP::IPBasedAnalyzer::AnalyzePacket(unsigned long, unsigned char const*, zeek::Packet*)
zeek::packet_analysis::Analyzer::ForwardPacket(unsigned long, unsigned char const*, zeek::Packet*, unsigned int) const
zeek::packet_analysis::IP::IPAnalyzer::AnalyzePacket(unsigned long, unsigned char const*, zeek::Packet*)
zeek::packet_analysis::Analyzer::ForwardPacket(unsigned long, unsigned char const*, zeek::Packet*, unsigned int) const
_ZNK4zeek15packet_analysis8Analyzer13Fo
rwardPacketEmPKhPNS_6PacketEj
zeek::packet_analysis::Analyzer::ForwardPacket(unsigned long, unsigned char const*, zeek::Packet*, unsigned int) const
zeek::packet_analysis::Analyzer::ForwardPacket(unsigned long, unsigned char const*, zeek::Packet*, unsigned int) const
zeek::packet_analysis::Manager::ProcessPacket(zeek::Packet*)
zeek::run_state::detail::dispatch_packet(zeek::Packet*, zeek::iosource::PktSrc*)
zeek::iosource::PktSrc::Process()
zeek::run_state::detail::run_loop()
mai
n
__libc_start_main
keithjjones commented 1 year ago

This is the function causing the segfault: https://github.com/cisagov/icsnpp-bacnet/blob/9e1f5455bfb0d003ac7b625cc88b456e5a4d0dc2/src/bacnet-analyzer.pac#L269

Kleinspider commented 1 year ago

I have added additional checks to that function based on number of BACnet tags and tag_lengths that could potentially cause a Seg fault. Unfortunately, without a packet capture to test on I am unable to confirm whether or not the latest commit has fixed the issue. Please let me know if you are still seeing these Seg faults with the newest commit/update.

Thanks!

keithjjones commented 1 year ago

This is awesome! Thank you! I've been running this for the past day and I haven't seen the segfault pop up. Thanks!