Project

General

Profile

Actions

Bug #13870

closed

pfSense-pkg-suricata-6.0.8_5 error in /usr/local/pkg/suricata/suricata.inc(4261)

Added by Brian Macy over 1 year ago. Updated over 1 year ago.

Status:
Resolved
Priority:
High
Assignee:
-
Category:
Suricata
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Plus Target Version:
Affected Version:
Affected Plus Version:
23.01
Affected Architecture:

Description

        // OpenVPN CSO
        foreach (config_get_path('openvpn/openvpn-csc', []) as $ovpnent) {
                if (is_array($ovpnent) && !config_path_enabled($ovpnent, 'disable')) {
                        if (!empty($ovpnent['tunnel_network'])) {

Not sure what the is supposed to be but config_path_enabled takes a string but is it be called only if $ovpnent is an array. This causes crashes constantly.

PHP ERROR: Type: 1, File: /etc/inc/config.lib.inc, Line: 1256, Message: Uncaught TypeError: config_path_enabled(): Argument #1 ($path) must be of type string, array given, called in /usr/local/pkg/suricata/suricata.inc on line 4261 and defined in /etc/inc/config.lib.inc:1256
Stack trace:
#0 /usr/local/pkg/suricata/suricata.inc(4261): config_path_enabled(Array, 'disable')
#1 /usr/local/pkg/suricata/suricata.inc(576): suricata_get_vpns_list()
#2 /usr/local/pkg/suricata/suricata_generate_yaml.php(46): suricata_build_list(Array, 'default')
#3 /usr/local/pkg/suricata/suricata_post_install.php(168): include('/usr/local/pkg/...')
#4 /etc/inc/pkg-utils.inc(781) : eval()'d code(1): include_once('/usr/local/pkg/...')
#5 /etc/inc/pkg-utils.inc(781): eval()
#6 /etc/inc/pkg-utils.inc(899): eval_once('include_once("/...')
#7 /etc/rc.packages(76): install_package_xml('suricata')
#8 {main}
thrown @ 2023-01-14 05:45:35

Actions #1

Updated by Bill Meeks over 1 year ago

This code is part of a function added by Viktor Gurov in the recent past. The purpose of the function is to collect all the VPNs configured on the firewall so their address subnets can be added to both the automatic default pass list and to any user-configured custom pass list (when that box is checked on the edit custom Pass List tab).

I can look into the code and try to figure out a solution, but someone on the Netgate team familiar with the VPN configuration process might could find a solution faster.

P.S. - this exact function is included in the Snort package in file /usr/local/pkg/snort/snort.inc , so Snort will experience the same issue when OpenVPN connections are configured. never mind, checked and Snort package is fine. Suricata package was a typo that I failed to catch and correct in an earlier pass.

Actions #2

Updated by Kris Phillips over 1 year ago

  • Status changed from New to Incomplete

I'm unable to reproduce any bad interactions between Suricata and OpenVPN. I created an OpenVPN interface, enabled it, enabled Suricata, and was unable to reproduce a crash.

What are the steps to reproduce this issue?

Actions #3

Updated by Brian Macy over 1 year ago

I honestly don't know how to cause it to happen... I don't know this stuff to even find out where in the config ovpnent gets its value from, but the code is clearly wrong. Unless the first parameter of config_path_enabled is NOT a string and somehow the RC messed that up.

But PHP will error on that line.
if ( is_array ($ovpnent) && !config_path_enabled( $ovpnent , 'disable'))

config_path_enabled requires the first parameter to be a string yet it ONLY gets called if $ovpnent is an Array. If that was an OR instead of an AND condition it would make much more sense.

Actions #4

Updated by Jim Pingle over 1 year ago

That should probably be something like this instead:

        foreach (config_get_path('openvpn/openvpn-csc', []) as $idx => $ovpnent) {
                if (is_array($ovpnent) && !config_path_enabled("openvpn/openvpn-csc/{$idx}", 'disable')) {
                        if (!empty($ovpnent['tunnel_network'])) {
Actions #5

Updated by Bill Meeks over 1 year ago

Jim Pingle wrote in #note-4:

That should probably be something like this instead:

[...]

Thanks Jim. I believe you are correct. There are likely other similar sections in that function. I will make the change at the point listed, and also check the remainder of the function for the same issue. Will get a pull request submitted today.

Edit: after looking at some of the other sub-sections in the function, I think this will also work. It is also how the other code sections handle a similar situation:

foreach (config_get_path('openvpn/openvpn-csc', []) as $idx => $ovpnent) {
                if (is_array($ovpnent) && !array_path_enabled($ovpnent, 'disable')) {
                        if (!empty($ovpnent['tunnel_network'])) {

Looking back jogged my memory, and this is probably a section I typo'd when making the PHP-8.1 updates back in October.

Pull Request submitted to correct this problem here: https://github.com/pfsense/FreeBSD-ports/pull/1214. This issue can be marked "resolved" when the pull request is merged.

Actions #6

Updated by Bill Meeks over 1 year ago

Pull request https://github.com/pfsense/FreeBSD-ports/pull/1214 has been merged. This issue may be marked as "Resolved".

Actions #7

Updated by Jim Pingle over 1 year ago

  • Status changed from Incomplete to Resolved
Actions

Also available in: Atom PDF