* [PATCH] BUG10940: remove leading zeros in ip address
@ 2015-11-09 11:42 Alexander Marx
2015-11-09 18:33 ` Michael Tremer
0 siblings, 1 reply; 3+ messages in thread
From: Alexander Marx @ 2015-11-09 11:42 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 4988 bytes --]
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'} =&General::ip2dec($fwhostsettings{'IP'});
- $fwhostsettings{'IP'} =&General::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::ip2dec($fwhostsettings{'IP'});
- $fwhostsettings{'IP'}=&General::dec2ip($fwhostsettings{'IP'});
$customhost{$key}[2] = $fwhostsettings{'IP'}."/".&General::iporsubtodec($fwhostsettings{'SUBNET'});
}else{
$customhost{$key}[2] = $fwhostsettings{'IP'};
--
1.9.1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] BUG10940: remove leading zeros in ip address
2015-11-09 11:42 [PATCH] BUG10940: remove leading zeros in ip address Alexander Marx
@ 2015-11-09 18:33 ` Michael Tremer
0 siblings, 0 replies; 3+ messages in thread
From: Michael Tremer @ 2015-11-09 18:33 UTC (permalink / raw)
To: development
[-- 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 --]
^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <5640F09E.6070708@oab.de>]
* Re: [PATCH] BUG10940: remove leading zeros in ip address
[not found] <5640F09E.6070708@oab.de>
@ 2015-11-09 22:16 ` Michael Tremer
0 siblings, 0 replies; 3+ messages in thread
From: Michael Tremer @ 2015-11-09 22:16 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 7021 bytes --]
On Mon, 2015-11-09 at 20:14 +0100, Alexander Marx wrote:
>
> FULL ACK, Michael.
>
> But unfortunately it seems not to be possible to catch that issue
> with modification of central functions like check_ip_address, becaus
> ethat functions only return true or false not the "normalized ip"
> without zero's.
>
> I am open for better solutions.... anyone ideas?!
It is too late for that now. I think the best option would simply to
consider those inputs as invalid and maybe fix this at some places
where we can and where we find this makes sense.
-Michael
P.S. Please make sure to reply to the list. Especially if you are
asking a question to the group :)
>
> cheers,
>
> Alex
>
> Am 09.11.2015 um 19:33 schrieb Michael Tremer:
> > 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'}."/".$fwhostsetti
> > > ngs{
> > > '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'} =&G
> > > ener
> > > al::ip2dec($fwhostsettings{'IP'});
> > > - $fwhostsettings{'IP'} =&G
> > > ener
> > > al::dec2ip($fwhostsettings{'IP'});
> > > $customnetwork{$key}[1] =
> > > &General::getnetworkip($fwhostsettings{'IP'},$fwhostsettings{'SUB
> > > NET'
> > > }) ;
> > > $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 --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-11-09 22:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-09 11:42 [PATCH] BUG10940: remove leading zeros in ip address Alexander Marx
2015-11-09 18:33 ` Michael Tremer
[not found] <5640F09E.6070708@oab.de>
2015-11-09 22:16 ` Michael Tremer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox