This patch is fine.

On Sun, 2018-02-11 at 19:51 +0100, Bernhard Held wrote:

$temp[1] might end with a newline; this is unavoidable when specifying serveral subnets. Thus, 'chomp $temp[1];' has to be moved before the comparisons with the green and blue subnets. Otherwise the comparison might always be true due to the newline.

---

html/cgi-bin/proxy.cgi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/html/cgi-bin/proxy.cgi b/html/cgi-bin/proxy.cgi index 6aa14e15a..ea3b41126 100644 --- a/html/cgi-bin/proxy.cgi +++ b/html/cgi-bin/proxy.cgi @@ -3064,12 +3064,12 @@ END foreach (@templist) { @temp = split(///);

• 		chomp $temp[1];
  	if (
  		($temp[0] ne $netsettings{'GREEN_NETADDRESS'}) && ($temp[1] ne $netsettings{'GREEN_NETMASK'}) &&
  		($temp[0] ne $netsettings{'BLUE_NETADDRESS'}) && ($temp[1] ne $netsettings{'BLUE_NETMASK'})
  		)
  	{
  

• 			chomp $temp[1];
  		print FILE " ||\n     (isInNet(myIpAddress(), \"$temp[0]\", \"$temp[1]\"))";
  	}
  }
  

I think you should better use &Network::equals() from /var/ipfire/network-functions.pl. This will take care of converting subnet masks to prefix notation and vice-versa.

Best, -Michael

On Sun, 2018-02-11 at 19:51 +0100, Bernhard Held wrote:

The logic of subnet comparison is broken. E.g. if the blue netmask is 255.255.255.0, it's impossible to add a VPN subnet with the same netmask. The proposed patch compares the subnets individually.

---

html/cgi-bin/proxy.cgi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/html/cgi-bin/proxy.cgi b/html/cgi-bin/proxy.cgi index ea3b41126..4993dde86 100644 --- a/html/cgi-bin/proxy.cgi +++ b/html/cgi-bin/proxy.cgi @@ -3066,8 +3066,8 @@ END @temp = split(///); chomp $temp[1]; if (

• 			($temp[0] ne $netsettings{'GREEN_NETADDRESS'}) && ($temp[1] ne $netsettings{'GREEN_NETMASK'}) &&
  

• 			($temp[0] ne $netsettings{'BLUE_NETADDRESS'}) && ($temp[1] ne $netsettings{'BLUE_NETMASK'})
  

• 			(($temp[0] ne $netsettings{'GREEN_NETADDRESS'}) || ($temp[1] ne $netsettings{'GREEN_NETMASK'})) &&
  

• 			(($temp[0] ne $netsettings{'BLUE_NETADDRESS'}) || ($temp[1] ne $netsettings{'BLUE_NETMASK'}))
  		)
  	{
  		print FILE " ||\n     (isInNet(myIpAddress(), \"$temp[0]\", \"$temp[1]\"))";