Same as the issue you reported for the Snort package, I don't believe this is a bug in the Suricata package source code. Instead I think it more likely is an issue with quoting/escaping the search term in your SID managment configuration file.

The actual line of PHP code being executed at line 2473 is this one:

if (preg_match($regex, $v['rule'])) {

The $regex variable in the above code is simply the verbatim token text stripped out of the SID management configuration file. I will need you to post the exact content of the file to determine what the problem may be.

Thanks for checking, Bill. These patterns worked OK in 2.4.4-p3 before the 2.4.5-RC upgrade. I do see a pattern typo in enablesid and dropsid which would affect the match but not be interpreted as a syntax error.

enablesid¶

# enablesid.conf
# classification and classtype rules should mirror dropsid list

# Specific types of rules to enable
pcre:\stls\s.*msg:.ET\sEXPLOIT
pcre:msg:.(ET\sEXPLOIT|SERVER-OTHER).*(OpenVPN|OpenSSL|TLS)

# Classification rules
# Selector order: classtype: / deployment / severity
#pcre:deployment\s+(Perimeter|Internet)
#pcre:deployment\s+(Datacenter|Internal)
#pcre:deployment\s+(SSLDecrypt)
#pcre:signature_severity\s+(Critical|Major|Minor)

# Select Datacenter/Internal Critical/Major rules
pcre:metadata:.*deployment\s+(Datacenter/Internal).*signature_severity\s+(Critical|Major)

# High Priority Classtype
pcre:classtype:attempted-(admin|user)
pcre:classtype:successful-(admin|user)
pcre:classtype:unsuccessful-user
pcre:classtype:shellcode-detect
pcre:classtype:trojan-activity
pcre:classtype:web-application-attack
#pcre:classtype:inappropriate-content
#pcre:classtype:policy-violation

# Medium Priority Classtype
pcre:classtype:network-scan
pcre:classtype:attempted-(dos|recon)
pcre:classtype:successful-(dos|recon-largescale|recon-limited)
pcre:classtype:(default-login-attempt|denial-of-service)
pcre:classtype:(bad-unknown|misc-attack)
#pcre:classtype:(non-standard-protocol|rpc-portmap-decode)
#pcre:classtype:(suspicious-filename-detect|suspicious-login)
#pcre:classtype:system-call-detect
#pcre:classtype:unusual-client-port-connection
#pcre:classtype:web-application-activity

dropsid¶

# Note: This file is used to specify what rules you wish to be set to have
# an action of drop rather than alert.

# classification and classtype rules should mirror enablesid list

# Classification rules
# Selector order: classtype: / deployment / severity
#pcre:deployment\s+(Perimeter|Internet)
#pcre:deployment\s+(Datacenter|Internal)
#pcre:deployment\s+(SSLDecrypt)
#pcre:signature_severity\s+(Critical|Major|Minor)

# Select Datacenter/Internal Critical/Major rules
pcre:metadata:.*deployment\s+(Datacenter/Internal).*signature_severity\s+(Critical|Major)

# High Priority Classtype
pcre:classtype:attempted-(admin|user)
pcre:classtype:successful-(admin|user)
pcre:classtype:unsuccessful-user
pcre:classtype:shellcode-detect
pcre:classtype:trojan-activity
pcre:classtype:web-application-attack
#pcre:classtype:inappropriate-content
#pcre:classtype:policy-violation

# Medium Priority Classtype
pcre:classtype:network-scan
pcre:classtype:attempted-(dos|recon)
pcre:classtype:successful-(dos|recon-largescale|recon-limited)
pcre:classtype:(default-login-attempt|denial-of-service)
#pcre:classtype:(bad-unknown|misc-attack)
#pcre:classtype:(non-standard-protocol|rpc-portmap-decode)
#pcre:classtype:(suspicious-filename-detect|suspicious-login)
#pcre:classtype:system-call-detect
#pcre:classtype:unusual-client-port-connection
#pcre:classtype:web-application-activity

# Malicious
#emerging-attack_response
#emerging-current_events
#emerging-dns
#emerging-dos
#emerging-exploit
#emerging-malware
#emerging-mobile_malware
#emerging-shellcode
#emerging-trojan
#emerging-web_client
#emerging-worm

# Block Lists
emerging-botcc.portgrouped
emerging-botcc
emerging-ciarmy
emerging-compromised-BLOCK
emerging-compromised
emerging-drop
emerging-dshield
emerging-rbn-malvertisers
emerging-rbn
emerging-tor

disable¶

# disable.conf
### Disable unused services
# pcre:(DNP3|ENIP|MODBUS)_(CLIENT|SERVER)
# pcre:(AIM|DNS|FTP|HTTP|SMTP|SQL|SSH|TELNET)_SERVERS
# pcre:(DNP3|FILE_DATA|FTP|HTTP|ORACLE|SSH)_PORTS

### Rule categories
# match string not containing word: ^((?!hede).)*$
# FIXME: This pattern was built for outside interface
# pcre:->((?!\s(22|500|4500|1194|ssh|\$SSH_PORTS)\s).)*(ET\sEXPLOIT|SERVER-OTHER)((?!\s(isakmp|openssh|openvpn)\s).)*$

### Rule Subsets
# pcre:classtype:not-suspicious

### ET POLICY
cve:2015-0204 #ET POLICY FREAK Weak Export Suite From Client
1:2002752 #ET POLICY Reserved Internal IP Traffic

### ET SCAN
1:2000538 #ET SCAN NMAP -sA (1)

### ET SHELLCODE
1:2013148 #ET SHELLCODE JavaScript Redefinition of a HeapLib Object - Likely Malicious Heap Spray Attempt
1:2012510 #ET SHELLCODE UTF-8/16 Encoded Shellcode

### ET TROJAN
1:2009205 #ET TROJAN Possible Downadup/Conficker-C P2P encrypted traffic UDP Ping Packet (bit value 1)
1:2009206 #ET TROJAN Possible Downadup/Conficker-C P2P encrypted traffic UDP Ping Packet (bit value 4)
1:2009207 #ET TROJAN Possible Downadup/Conficker-C P2P encrypted traffic UDP Ping Packet (bit value 5)
1:2009208 #ET TROJAN Possible Downadup/Conficker-C P2P encrypted traffic UDP Ping Packet (bit value 16)

### SURICATA FALSE POSITIVES
### decoder-events.rules FPs
# Lots of noise
#1:2200003 #SURICATA IPv4 truncated packet
#1:2200007 #SURICATA IPv4 padding required
# Breaks IPv6 tunnels
#1:2200029 #SURICATA ICMPv6 unknown type
# Apple uses lots of odd options
#1:2200036 #SURICATA TCP option invalid length
# Loads of noise, DNS and others
#1:2200038 #SURICATA UDP packet too small
# Messes up some DNS traffic
#1:2200070 #SURICATA FRAG IPv4 Fragmentation overlap
#1:2200072 #SURICATA FRAG IPv6 Fragmentation overlap
# Messes up with DNS resolution on LAN
#1:2200073 #SURICATA IPv4 invalid checksum
# Bittorrent noise, DNS
#1:2200075 #SURICATA UDPv4 invalid checksum
#1:2200078 #SURICATA UDPv6 invalid checksum
# Lots of useless noise
#1:2200076 #SURICATA ICMPv4 invalid checksum
#1:2200079 #SURICATA ICMPv6 invalid checksum
# Messes with IPv6 DNS resolution with some DNS servers
#1:2200080 #SURICATA IPv6 useless Fragment extension header
# Other noise
#1:2200094 #SURICATA zero length padN option

### stream-events.rules FPs
#1:2210020 #SURICATA STREAM ESTABLISHED packet out of window
#1:2210023 #SURICATA STREAM ESTABLISHED SYNACK resend with different ACK
#1:2210029 #SURICATA STREAM ESTABLISHED invalid ack
#1:2210038 #SURICATA STREAM FIN out of window
#1:2210044 #SURICATA STREAM Packet with invalid timestamp
#1:2210045 #SURICATA STREAM Packet with invalid ack
#1:2210054 #SURICATA STREAM excessive retransmissions

### smtp-events.rules FPs
#1:2220000 #SURICATA SMTP invalid reply
#1:2220008 #SURICATA SMTP data command rejected

### http-events.rules FPs
# breaks Windows updates
#1:2221000 #SURICATA HTTP unknown error
# Random false positives
#1:2221001 #SURICATA HTTP gzip decompression failed
#1:2221010 #SURICATA HTTP unable to match response to request
#1:2221015 #SURICATA HTTP Host header ambiguous
#1:2221022 #SURICATA HTTP multipart generic error
#1:2221028 #SURICATA HTTP Host header invalid

### tls-events.rules FPs
#1:2230002 #SURICATA TLS invalid record type
#1:2230003 #SURICATA TLS invalid handshake message
#1:2230009 #SURICATA TLS error message encountered
#1:2230010 #SURICATA TLS invalid record/traffic
#1:2230015 #SURICATA TLS invalid record version

### dns-events.rules FPs
#1:2240001 #SURICATA DNS Unsolicited response
#1:2240002 #SURICATA DNS malformed request data
#1:2240003 #SURICATA DNS malformed response data

I think the issue is traced to the following line:

pcre:metadata:.*deployment\s+(Datacenter/Internal).*signature_severity\s+(Critical|Major)

Unlike snort, the suricata package doesn't escape PCRE patterns with preg_quote() before passing them to preg_match(). This means that unlike snort, PCRE patterns actually work with suricata. Also unlike snort, malformed PCRE patterns will throw errors.

Because of the way patterns are wrapped with '/<pattern>/i' before use in preg_match(), a side-effect is that inclusion of a '/' in a pattern will result in a malformed pattern. In this case the word "Internal" is being interpreted as a pattern modifier, resulting in this error.

I'm not sure why this configuration didn't throw an error in 2.4.4-p3.

John Silva wrote:

I think the issue is traced to the following line:

[...]

Unlike snort, the suricata package doesn't escape PCRE patterns with preg_quote() before passing them to preg_match(). This means that unlike snort, PCRE patterns actually work with suricata. Also unlike snort, malformed PCRE patterns will throw errors.

Because of the way patterns are wrapped with '/<pattern>/i' before use in preg_match(), a side-effect is that inclusion of a '/' in a pattern will result in a malformed pattern. In this case the word "Internal" is being interpreted as a pattern modifier, resulting in this error.

I'm not sure why this configuration didn't throw an error in 2.4.4-p3.

I think there was a reason why the preg_quote() was added to the Snort package, but it was a while back and the original reason escapes me at the moment. I could go research the Github history and probably refresh my memory.

I don't think there is an easy fix here because making the two GUI packages have the same coding in this area is likely to break some existing configurations (PCRE search configurations I'm talking about).

If I had to choose I'd choose to not use preg_quote() so that pcre works as expected.

I think this could be done safely by adding a sid config validation function that's called during package upgrade (to catch broken config) and when sid configs are saved (to provide immediate feedback). Existing configs might be broken but the user would at least be warned.

John Silva wrote:

If I had to choose I'd choose to not use preg_quote() so that pcre works as expected.

I think this could be done safely by adding a sid config validation function that's called during package upgrade (to catch broken config) and when sid configs are saved (to provide immediate feedback). Existing configs might be broken but the user would at least be warned.

I will readily admit to being a near dunce when it comes to PCRE stuff. I just have never used it a lot, and everytime I do use it I have to spend a while on Google refreshing my memory and looking for PCRE examples to copy and modify.

So are you suggesting that in Suricata (and presumably, later in Snort as well) that it would be best to remove everything from the preg_match() search parameter including the current '/<pattern>/i' wrapper and just use the raw PCRE coming in from the user's SID MGMT conf?

I think that forcing inclusion of the regex delimeter in the pcre: definition would be very flexible but would definitely break all existing use of the keyword. You could test for the presence of the leading delimiter and wrap the regex if it isn't present - that would preserve backward compatibility with existing configuration.

This is addressed by the latest posted versions of the Suricata GUI packages for pfSense 2.4.5-RC and 2.5-DEVEL. The chosen solution was to mimic the current Snort behavior by calling preg_quote() with the user-supplied pcre search string to insure any embedded '/' characters are properly escaped. The comments in the SID MGM configuration file examples state that when using the "pcre" keyword that inclusion of the '/' delimiters is NOT required.

This issue can be closed.