search

cisagov / icsnpp-bacnet

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

A missed bacnet detection? #9

Closed keithjjones closed 2 years ago

keithjjones commented 2 years ago

🐛 Summary

What's wrong? Please be specific.

To reproduce

Steps to reproduce the behavior:

I ran this logic on http://kargs.net/captures/who-has-I-have.cap, but it does not detect Bacnet. When I poked around in Wireshark, the pcap looks ok. I'm not sure why it's not detecting. Flagging here.

Expected behavior

What did you expect to happen that didn't?

I expected bacnet to be detected.

Kleinspider commented 2 years ago

Apologies for the delayed response to this issue. Is this still an issue with the current version of this parser? If so, would you let me know exactly which commands/packets are not being detected as BACnet, because I was able to parse that PCAP on my machine with the BACnet logs generated.

Thanks!

keithjjones commented 2 years ago

I can't remember what I originally tested it on. Probably my mac. To replicate, I tried it on my mac just now and got this:

(3) Keiths-MacBook-Pro-15:tmp keith.jones$ zkg install icsnpp-bacnet
The following packages will be INSTALLED:
  zeek/cisagov/icsnpp-bacnet (main)

Proceed? [Y/n] 
Running unit tests for "zeek/cisagov/icsnpp-bacnet"
Installing "zeek/cisagov/icsnpp-bacnet"...............
Installed "zeek/cisagov/icsnpp-bacnet" (main)
Loaded "zeek/cisagov/icsnpp-bacnet"
(3) Keiths-MacBook-Pro-15:tmp keith.jones$ zeek -Cr ~/Desktop/who-has-I-have.cap icsnpp-bacnet
Segmentation fault: 11
(3) Keiths-MacBook-Pro-15:tmp keith.jones$ zeek -Cr ~/Desktop/who-has-I-have.cap packages
Segmentation fault: 11
(3) Keiths-MacBook-Pro-15:tmp keith.jones$ zeek -Cr ~/Desktop/who-has-I-have.cap
(3) Keiths-MacBook-Pro-15:tmp keith.jones$

What type of system did you use to test it?

Kleinspider commented 2 years ago

This was an interesting one to track down. Looks like in the very first packet there was an extra BACnet tag appended to the end of the traffic (or just extra data captured. Wireshark just treats it as extra data and doesn't try and parse it). There was an if statement in function process_who_is that checked for tag size > 0 and then read from 2 tags leading to an issue when there was only 1 tag. This condition has been fixed with latest commit.

Kleinspider commented 2 years ago

I believe this has fixed this issue, but please let me know if any additional issues arise from this packet capture.