Re: [PATCH] BUG10940: remove leading zeros in ip address

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 5508 bytes --]

Hi,

this does not look like the cleanest solution to be honest, but I guess
this is acceptable. It solves the issue at hand, but generally I would
like to have this solved one time so that one does not have to keep
this problem in mind when writing new code.

Merged.

Best,
-Michael

On Mon, 2015-11-09 at 12:42 +0100, Alexander Marx wrote:
> in firewallgroups (hosts) an error was created when using ip adresses
> like 192.168.000.008. Now all leading zeros are deleted in
> firewallgroups and in the firewall itself when using single ip
> addresses
> as source or target.
> 
> Signed-off-by: Alexander Marx <alexander.marx(a)ipfire.org>
> ---
>  config/cfgroot/network-functions.pl | 13 +++++++++++++
>  html/cgi-bin/firewall.cgi           |  8 ++++++++
>  html/cgi-bin/fwhosts.cgi            | 13 +++++++------
>  3 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/config/cfgroot/network-functions.pl
> b/config/cfgroot/network-functions.pl
> index cb4ca3d..70fa5ed 100644
> --- a/config/cfgroot/network-functions.pl
> +++ b/config/cfgroot/network-functions.pl
> @@ -122,6 +122,19 @@ sub network2bin($) {
>  	return ($network_start, $netmask_bin);
>  }
>  
> +# Deletes leading zeros in ip address
> +sub ip_remove_zero{
> +	my $address = shift;
> +	my @ip = split (/\./, $address);
> +
> +	foreach my $octet (@ip) {
> +		$octet = int($octet);
> +	}
> +
> +	$address = join (".", @ip);
> +
> +	return $address;
> +}
>  # Returns True for all valid IP addresses
>  sub check_ip_address($) {
>  	my $address = shift;
> diff --git a/html/cgi-bin/firewall.cgi b/html/cgi-bin/firewall.cgi
> index 682c285..8007182 100644
> --- a/html/cgi-bin/firewall.cgi
> +++ b/html/cgi-bin/firewall.cgi
> @@ -31,6 +31,7 @@ no warnings 'uninitialized';
>  #use CGI::Carp 'fatalsToBrowser';
>  
>  require '/var/ipfire/general-functions.pl';
> +require '/var/ipfire/network-functions.pl';
>  require "${General::swroot}/lang.pl";
>  require "${General::swroot}/header.pl";
>  require "${General::swroot}/geoip-functions.pl";
> @@ -465,6 +466,9 @@ sub checksource
>  			}
>  		}
>  		if ($fwdfwsettings{'isip'} eq 'on'){
> +			#remove leading zero
> +			$ip = &Network::ip_remove_zero($ip);
> +
>  			##check if ip is valid
>  			if (! &General::validip($ip)){
>  				$errormessage.=$Lang::tr{'fwdfw err
> src_addr'}."<br>";
> @@ -569,11 +573,15 @@ sub checktarget
>  			($ip,$subnet)=split
> (/\//,$fwdfwsettings{'tgt_addr'});
>  			$subnet = &General::iporsubtocidr($subnet);
>  		}
> +
>  		#check if only ip
>  		if($fwdfwsettings{'tgt_addr'}=~/^(\d{1,3})\.(\d{1,3}
> )\.(\d{1,3})\.(\d{1,3})$/){
>  			$ip=$fwdfwsettings{'tgt_addr'};
>  			$subnet='32';
>  		}
> +		#remove leading zero
> +		$ip = &Network::ip_remove_zero($ip);
> +
>  		#check if ip is valid
>  		if (! &General::validip($ip)){
>  			$errormessage.=$Lang::tr{'fwdfw err
> tgt_addr'}."<br>";
> diff --git a/html/cgi-bin/fwhosts.cgi b/html/cgi-bin/fwhosts.cgi
> index 994a50a..35afad3 100644
> --- a/html/cgi-bin/fwhosts.cgi
> +++ b/html/cgi-bin/fwhosts.cgi
> @@ -27,6 +27,7 @@ use Sort::Naturally;
>  use CGI::Carp 'fatalsToBrowser';
>  no warnings 'uninitialized';
>  require '/var/ipfire/general-functions.pl';
> +require '/var/ipfire/network-functions.pl';
>  require "/var/ipfire/geoip-functions.pl";
>  require "/usr/lib/firewall/firewall-lib.pl";
>  require "${General::swroot}/lang.pl";
> @@ -277,6 +278,9 @@ if ($fwhostsettings{'ACTION'} eq 'savenet' )
>  		&addnet;
>  		&viewtablenet;
>  	}else{
> +		#convert ip if leading '0' exists
> +		$fwhostsettings{'IP'} =
> &Network::ip_remove_zero($fwhostsettings{'IP'});
> +
>  		#check valid ip 
>  		if
> (!&General::validipandmask($fwhostsettings{'IP'}."/".$fwhostsettings{
> 'SUBNET'}))
>  		{
> @@ -372,9 +376,6 @@ if ($fwhostsettings{'ACTION'} eq 'savenet' )
>  			foreach my $i (0 .. 3) {
> $customnetwork{$key}[$i] = "";}
>  			$fwhostsettings{'SUBNET'}	=
> &General::iporsubtocidr($fwhostsettings{'SUBNET'});
>  			$customnetwork{$key}[0] 	=
> $fwhostsettings{'HOSTNAME'};
> -			#convert ip when leading '0' in byte
> -			$fwhostsettings{'IP'}		=&Gener
> al::ip2dec($fwhostsettings{'IP'});
> -			$fwhostsettings{'IP'}		=&Gener
> al::dec2ip($fwhostsettings{'IP'});
>  			$customnetwork{$key}[1] 	=
> &General::getnetworkip($fwhostsettings{'IP'},$fwhostsettings{'SUBNET'
> }) ;
>  			$customnetwork{$key}[2] 	=
> &General::iporsubtodec($fwhostsettings{'SUBNET'}) ;
>  			$customnetwork{$key}[3] 	=
> $fwhostsettings{'NETREMARK'};
> @@ -423,6 +424,9 @@ if ($fwhostsettings{'ACTION'} eq 'savehost')
>  		}
>  		#CHECK IP-PART
>  		if ($fwhostsettings{'type'} eq 'ip'){
> +			#convert ip if leading '0' exists
> +			$fwhostsettings{'IP'} =
> &Network::ip_remove_zero($fwhostsettings{'IP'});
> +
>  			#check for subnet
>  			if (rindex($fwhostsettings{'IP'},'/') eq '
> -1' ){
>  				if($fwhostsettings{'type'} eq 'ip'
> && !&General::validipandmask($fwhostsettings{'IP'}."/32"))
> @@ -503,9 +507,6 @@ if ($fwhostsettings{'ACTION'} eq 'savehost')
>  			$customhost{$key}[0] =
> $fwhostsettings{'HOSTNAME'} ;
>  			$customhost{$key}[1] =
> $fwhostsettings{'type'} ;
>  			if ($fwhostsettings{'type'} eq 'ip'){
> -				#convert ip when leading '0' in byte
> -				$fwhostsettings{'IP'}=&General::ip2d
> ec($fwhostsettings{'IP'});
> -				$fwhostsettings{'IP'}=&General::dec2
> ip($fwhostsettings{'IP'});
>  				$customhost{$key}[2] =
> $fwhostsettings{'IP'}."/".&General::iporsubtodec($fwhostsettings{'SUB
> NET'});
>  			}else{
>  				$customhost{$key}[2] =
> $fwhostsettings{'IP'};

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]