Hi,
just an annotation, see below

> Gesendet: Mittwoch, 17. Februar 2021 um 14:58 Uhr
> Von: "Adolf Belka" <adolf.belka(a)ipfire.org>
> An: development(a)lists.ipfire.org
> Betreff: [PATCH] bug#10629: Prevent dynamic and fixed leases overlapping
>
> - This is a fix for bug #10629
> - I have tested this out on my vm testbed system. Everything worked fine
>   with this. It would be good to get other test feedback in case I have
>   missed something.
> - This fix flags up if a fixed lease is created within the existing dynamic
>   range
> - This fix also works if a dynamic lease is converted to a fixed lease. A
>   new IP outside the dynamic range has to be selected.
> - A check has also been added if the dynamic range is modified to overlap
>   any existing fixed leases. The error message will also inform how many
>   fixed leases are now overlapped by the modified dynamic range.
> - If an interface is disabled and fixed leases within the dynamic range
>   created or the dynamic range expanded to overlap with existing fixed
>   leases, then when the interface is enabled again the check is carried
>   out and catches these and prevents them being set.
> - New error messages added to en.pl file
>
> Signed-off-by: Adolf Belka <adolf.belka(a)ipfire.org>
> ---
>  config/cfgroot/general-functions.pl | 18 ++++++++++++
>  doc/language_issues.de              |  2 ++
>  doc/language_issues.en              |  2 ++
>  doc/language_issues.es              |  2 ++
>  doc/language_issues.fr              |  2 ++
>  doc/language_issues.it              |  2 ++
>  doc/language_issues.nl              |  2 ++
>  doc/language_issues.pl              |  2 ++
>  doc/language_issues.ru              |  2 ++
>  doc/language_issues.tr              |  2 ++
>  doc/language_missings               | 24 ++++++++++++++++
>  html/cgi-bin/dhcp.cgi               | 43 +++++++++++++++++++++++++++++
>  langs/en/cgi-bin/en.pl              |  3 ++
>  13 files changed, 106 insertions(+)
>
> diff --git a/config/cfgroot/general-functions.pl b/config/cfgroot/general-functions.pl
> index a6656ccf5..a8c8d171c 100644
> --- a/config/cfgroot/general-functions.pl
> +++ b/config/cfgroot/general-functions.pl
> @@ -591,6 +591,24 @@ sub check_net_internal_exact{
>  	if (($ownnet{'RED_NETADDRESS'} 		ne '' && $ownnet{'RED_NETADDRESS'} 		ne '0.0.0.0') && &Network::network_equal("$ownnet{'RED_NETADDRESS'}/$ownnet{'RED_NETMASK'}", $network)){ $errormessage=$Lang::tr{'ccd err red'};return $errormessage;}
>  }
>
> +sub ip_address_in_ip_range($$) {
> +# Returns True if $ipaddress is within $ipstart and $ipend range.
> +	my $ipaddress = shift;
> +	my $ipstart = shift;
> +	my $ipend = shift;
> +
> +	my $ipaddress_bin = &Network::ip2bin($ipaddress);
> +	return undef unless (defined $ipaddress_bin);
> +
> +	my $ipstart_bin = &Network::ip2bin($ipstart);
> +	return undef unless (defined $ipstart_bin);
> +
> +	my $ipend_bin = &Network::ip2bin($ipend);
> +	return undef unless (defined $ipend_bin);
> +
> +	return (($ipaddress_bin >= $ipstart_bin) && ($ipaddress_bin <= $ipend_bin));
> +}
> +
>  sub validport
>  {
>  	$_ = $_[0];
> diff --git a/doc/language_issues.de b/doc/language_issues.de
> index 5d079036a..cb3e89b2e 100644
> --- a/doc/language_issues.de
> +++ b/doc/language_issues.de
> @@ -840,6 +840,8 @@ WARNING: translation string unused: zoneconf val vlan amount assignment error
>  WARNING: translation string unused: zoneconf val vlan tag assignment error
>  WARNING: translation string unused: zoneconf val zoneslave amount error
>  WARNING: untranslated string: desired = Desired
> +WARNING: untranslated string: dhcp dynamic range overlap = Dynamic range overlapped with
> +WARNING: untranslated string: dhcp fixed ip address =  Fixed IP Address(es)
>  WARNING: untranslated string: disable = Disable
>  WARNING: untranslated string: enable = Enable
>  WARNING: untranslated string: error the to date has to be later than the from date = The to date has to be later than the from date!
> diff --git a/doc/language_issues.en b/doc/language_issues.en
> index 6e30eb995..832ff8d92 100644
> --- a/doc/language_issues.en
> +++ b/doc/language_issues.en
> @@ -582,6 +582,8 @@ WARNING: untranslated string: dhcp dns key name = Key Name
>  WARNING: untranslated string: dhcp dns update = DNS Update
>  WARNING: untranslated string: dhcp dns update algo = Algorithm
>  WARNING: untranslated string: dhcp dns update secret = Secret
> +WARNING: untranslated string: dhcp dynamic range overlap = Dynamic range overlapped with
> +WARNING: untranslated string: dhcp fixed ip address =  Fixed IP Address(es)
>  WARNING: untranslated string: dhcp server = DHCP Server
>  WARNING: untranslated string: dhcp server disabled = DHCP server disabled.  Stopped.
>  WARNING: untranslated string: dhcp server enabled = DHCP server enabled.  Restarting.
> diff --git a/doc/language_issues.es b/doc/language_issues.es
> index 82d65d99c..b65ecd164 100644
> --- a/doc/language_issues.es
> +++ b/doc/language_issues.es
> @@ -893,6 +893,8 @@ WARNING: untranslated string: dhcp dns key name = Key Name
>  WARNING: untranslated string: dhcp dns update = DNS Update
>  WARNING: untranslated string: dhcp dns update algo = Algorithm
>  WARNING: untranslated string: dhcp dns update secret = Secret
> +WARNING: untranslated string: dhcp dynamic range overlap = Dynamic range overlapped with
> +WARNING: untranslated string: dhcp fixed ip address =  Fixed IP Address(es)
>  WARNING: untranslated string: dhcp valid range required when deny known clients checked = Valid range required when "Deny known clients:" is checked
>  WARNING: untranslated string: disable = Disable
>  WARNING: untranslated string: disconnected = Disconnected
> diff --git a/doc/language_issues.fr b/doc/language_issues.fr
> index 942be73ec..71de90bd7 100644
> --- a/doc/language_issues.fr
> +++ b/doc/language_issues.fr
> @@ -880,6 +880,8 @@ WARNING: translation string unused: zoneconf val vlan amount assignment error
>  WARNING: translation string unused: zoneconf val vlan tag assignment error
>  WARNING: translation string unused: zoneconf val zoneslave amount error
>  WARNING: untranslated string: dhcp deny known clients: = Deny known clients:
> +WARNING: untranslated string: dhcp dynamic range overlap = Dynamic range overlapped with
> +WARNING: untranslated string: dhcp fixed ip address =  Fixed IP Address(es)
>  WARNING: untranslated string: dhcp valid range required when deny known clients checked = Valid range required when "Deny known clients:" is checked
>  WARNING: untranslated string: fwhost cust locationgrp = unknown string
>  WARNING: untranslated string: fwhost err hostip = unknown string
> diff --git a/doc/language_issues.it b/doc/language_issues.it
> index 98074e59f..a4cd8c5db 100644
> --- a/doc/language_issues.it
> +++ b/doc/language_issues.it
> @@ -917,6 +917,8 @@ WARNING: untranslated string: dhcp dns key name = Key Name
>  WARNING: untranslated string: dhcp dns update = DNS Update
>  WARNING: untranslated string: dhcp dns update algo = Algorithm
>  WARNING: untranslated string: dhcp dns update secret = Secret
> +WARNING: untranslated string: dhcp dynamic range overlap = Dynamic range overlapped with
> +WARNING: untranslated string: dhcp fixed ip address =  Fixed IP Address(es)
>  WARNING: untranslated string: dhcp valid range required when deny known clients checked = Valid range required when "Deny known clients:" is checked
>  WARNING: untranslated string: disable = Disable
>  WARNING: untranslated string: disconnected = Disconnected
> diff --git a/doc/language_issues.nl b/doc/language_issues.nl
> index 8eebbd57f..9cef4790e 100644
> --- a/doc/language_issues.nl
> +++ b/doc/language_issues.nl
> @@ -918,6 +918,8 @@ WARNING: untranslated string: dhcp dns key name = Key Name
>  WARNING: untranslated string: dhcp dns update = DNS Update
>  WARNING: untranslated string: dhcp dns update algo = Algorithm
>  WARNING: untranslated string: dhcp dns update secret = Secret
> +WARNING: untranslated string: dhcp dynamic range overlap = Dynamic range overlapped with
> +WARNING: untranslated string: dhcp fixed ip address =  Fixed IP Address(es)
>  WARNING: untranslated string: disable = Disable
>  WARNING: untranslated string: disconnected = Disconnected
>  WARNING: untranslated string: dl client arch insecure = Download insecure Client Package (zip)
> diff --git a/doc/language_issues.pl b/doc/language_issues.pl
> index 82d65d99c..b65ecd164 100644
> --- a/doc/language_issues.pl
> +++ b/doc/language_issues.pl
> @@ -893,6 +893,8 @@ WARNING: untranslated string: dhcp dns key name = Key Name
>  WARNING: untranslated string: dhcp dns update = DNS Update
>  WARNING: untranslated string: dhcp dns update algo = Algorithm
>  WARNING: untranslated string: dhcp dns update secret = Secret
> +WARNING: untranslated string: dhcp dynamic range overlap = Dynamic range overlapped with
> +WARNING: untranslated string: dhcp fixed ip address =  Fixed IP Address(es)
>  WARNING: untranslated string: dhcp valid range required when deny known clients checked = Valid range required when "Deny known clients:" is checked
>  WARNING: untranslated string: disable = Disable
>  WARNING: untranslated string: disconnected = Disconnected
> diff --git a/doc/language_issues.ru b/doc/language_issues.ru
> index 43c1f8c08..76fd6b350 100644
> --- a/doc/language_issues.ru
> +++ b/doc/language_issues.ru
> @@ -895,6 +895,8 @@ WARNING: untranslated string: dhcp dns key name = Key Name
>  WARNING: untranslated string: dhcp dns update = DNS Update
>  WARNING: untranslated string: dhcp dns update algo = Algorithm
>  WARNING: untranslated string: dhcp dns update secret = Secret
> +WARNING: untranslated string: dhcp dynamic range overlap = Dynamic range overlapped with
> +WARNING: untranslated string: dhcp fixed ip address =  Fixed IP Address(es)
>  WARNING: untranslated string: dhcp valid range required when deny known clients checked = Valid range required when "Deny known clients:" is checked
>  WARNING: untranslated string: disable = Disable
>  WARNING: untranslated string: disconnected = Disconnected
> diff --git a/doc/language_issues.tr b/doc/language_issues.tr
> index 439a58890..bd78a5a4e 100644
> --- a/doc/language_issues.tr
> +++ b/doc/language_issues.tr
> @@ -896,6 +896,8 @@ WARNING: untranslated string: dangerous = Dangerous
>  WARNING: untranslated string: default IP address = Default IP Address
>  WARNING: untranslated string: desired = Desired
>  WARNING: untranslated string: dhcp deny known clients: = Deny known clients:
> +WARNING: untranslated string: dhcp dynamic range overlap = Dynamic range overlapped with
> +WARNING: untranslated string: dhcp fixed ip address =  Fixed IP Address(es)
>  WARNING: untranslated string: dhcp valid range required when deny known clients checked = Valid range required when "Deny known clients:" is checked
>  WARNING: untranslated string: disable = Disable
>  WARNING: untranslated string: disconnected = Disconnected
> diff --git a/doc/language_missings b/doc/language_missings
> index 0d89426ca..3d6c5103d 100644
> --- a/doc/language_missings
> +++ b/doc/language_missings
> @@ -28,6 +28,9 @@
>  < could not connect to www ipfire org
>  < cryptographic settings
>  < desired
> +< dhcp dynamic range overlap
> +< dhcp fixed ip address
> +< dhcp fixed ip address in dynamic range
>  < dhcp server disabled on blue interface
>  < dhcp server enabled on blue interface
>  < dh name is invalid
> @@ -230,6 +233,9 @@
>  < dhcp dns update
>  < dhcp dns update algo
>  < dhcp dns update secret
> +< dhcp dynamic range overlap
> +< dhcp fixed ip address
> +< dhcp fixed ip address in dynamic range
>  < dhcp valid range required when deny known clients checked
>  < dh key move failed
>  < dh key warn
> @@ -969,6 +975,9 @@
>  < bewan adsl pci st
>  < bewan adsl usb
>  < dhcp deny known clients:
> +< dhcp dynamic range overlap
> +< dhcp fixed ip address
> +< dhcp fixed ip address in dynamic range
>  < dhcp valid range required when deny known clients checked
>  < g.dtm
>  < g.lite
> @@ -1071,6 +1080,9 @@
>  < dhcp dns update
>  < dhcp dns update algo
>  < dhcp dns update secret
> +< dhcp dynamic range overlap
> +< dhcp fixed ip address
> +< dhcp fixed ip address in dynamic range
>  < dhcp valid range required when deny known clients checked
>  < disable
>  < Disabled
> @@ -1460,6 +1472,9 @@
>  < dhcp dns update
>  < dhcp dns update algo
>  < dhcp dns update secret
> +< dhcp dynamic range overlap
> +< dhcp fixed ip address
> +< dhcp fixed ip address in dynamic range
>  < dh key move failed
>  < dh key warn
>  < dh key warn1
> @@ -1965,6 +1980,9 @@
>  < dhcp dns update
>  < dhcp dns update algo
>  < dhcp dns update secret
> +< dhcp dynamic range overlap
> +< dhcp fixed ip address
> +< dhcp fixed ip address in dynamic range
>  < dhcp valid range required when deny known clients checked
>  < dh key move failed
>  < dh key warn
> @@ -2848,6 +2866,9 @@
>  < dhcp dns update
>  < dhcp dns update algo
>  < dhcp dns update secret
> +< dhcp dynamic range overlap
> +< dhcp fixed ip address
> +< dhcp fixed ip address in dynamic range
>  < dhcp valid range required when deny known clients checked
>  < dh key move failed
>  < dh key warn
> @@ -3595,6 +3616,9 @@
>  < default IP address
>  < desired
>  < dhcp deny known clients:
> +< dhcp dynamic range overlap
> +< dhcp fixed ip address
> +< dhcp fixed ip address in dynamic range
>  < dhcp valid range required when deny known clients checked
>  < disable
>  < Disabled
> diff --git a/html/cgi-bin/dhcp.cgi b/html/cgi-bin/dhcp.cgi
> index 867614f2a..82ea754c7 100644
> --- a/html/cgi-bin/dhcp.cgi
> +++ b/html/cgi-bin/dhcp.cgi
> @@ -130,6 +130,7 @@ open(FILE, "$filename2") or die 'Unable to open fixed leases file.';
>  our @current2 = <FILE>;
>  close(FILE);
>
> +
>  # Check Settings1 first because they are needed by &buildconf
>  if ($dhcpsettings{'ACTION'} eq $Lang::tr{'save'}) {
>      foreach my $itf (@ITFs) {
> @@ -183,6 +184,24 @@ if ($dhcpsettings{'ACTION'} eq $Lang::tr{'save'}) {
>  		}
>  	    }
>
> +	    # Check if dynamic range and Fixed IP Addresses overlap
> +	    if ((!$dhcpsettings{"START_ADDR_${itf}"}) eq '' && (!$dhcpsettings{"END_ADDR_${itf}"}) eq '') {

Shouldn't this read
            if (($dhcpsettings{"START_ADDR_${itf}"}) ne '' && ($dhcpsettings{"END_ADDR_${itf}"}) ne '') {

- Bernhard
> +		my $count=0;
> +		foreach my $line (@current2) {
> +			chomp($line);
> +			my @temp = split(/\,/,$line);
> +			if (&General::ip_address_in_ip_range($temp[1],
> +							     $dhcpsettings{"START_ADDR_${itf}"},
> +							     $dhcpsettings{"END_ADDR_${itf}"})) {
> +				$count++;
> +			}
> +		}
> +		if ($count > 0) {
> +			$errormessage = "DHCP on ${itf}: " . $Lang::tr{'dhcp dynamic range overlap'} . $count . $Lang::tr{'dhcp fixed ip address'};
> +			goto ERROR;
> +		}
> +	    }
> +
>  	    if (!($dhcpsettings{"DEFAULT_LEASE_TIME_${itf}"} =~ /^\d+$/)) {
>  		$errormessage = "DHCP on ${itf}: " . $Lang::tr{'invalid default lease time'} . $dhcpsettings{'DEFAULT_LEASE_TIME_${itf}'};
>  		goto ERROR;
> @@ -415,10 +434,34 @@ if ($dhcpsettings{'ACTION'} eq $Lang::tr{'toggle enable disable'}.'2') {
>  if ($dhcpsettings{'ACTION'} eq $Lang::tr{'add'}.'2') {
>      $dhcpsettings{'FIX_MAC'} =~ tr/-/:/;
>      unless(&General::validip($dhcpsettings{'FIX_ADDR'})) { $errormessage = $Lang::tr{'invalid fixed ip address'}; }
> +# Check if fixed address is in the dynamic range, if defined
> +    foreach my $itf (@ITFs) {
> +	if ($dhcpsettings{"ENABLE_${itf}"} eq 'on' ) {
> +		if ($dhcpsettings{"START_ADDR_${itf}"}) {
> +    			if (&General::ip_address_in_ip_range($dhcpsettings{'FIX_ADDR'},
> +							     $dhcpsettings{"START_ADDR_${itf}"},
> +							     $dhcpsettings{"END_ADDR_${itf}"})) {
> +				$errormessage = $Lang::tr{"dhcp fixed ip address in dynamic range"};
> +			}
> +		}
> +	}
> +    }
>      unless(&General::validmac($dhcpsettings{'FIX_MAC'})) { $errormessage = $Lang::tr{'invalid fixed mac address'}; }
>      if ($dhcpsettings{'FIX_NEXTADDR'}) {
>          unless(&General::validip($dhcpsettings{'FIX_NEXTADDR'})) { $errormessage = $Lang::tr{'invalid fixed ip address'}; }
>      }
> +# Check if fixed next address is in the dynamic range, if defined
> +    foreach my $itf (@ITFs) {
> +	if ($dhcpsettings{"ENABLE_${itf}"} eq 'on' ) {
> +		if ($dhcpsettings{"START_ADDR_${itf}"}) {
> +    			if (&General::ip_address_in_ip_range($dhcpsettings{'FIX_NEXTADDR'},
> +							     $dhcpsettings{"START_ADDR_${itf}"},
> +							     $dhcpsettings{"END_ADDR_${itf}"})) {
> +				$errormessage = $Lang::tr{"dhcp fixed ip address in dynamic range"};
> +			}
> +		}
> +	}
> +    }
>
>      my $key = 0;
>      CHECK:foreach my $line (@current2) {
> diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
> index 95a1cfda4..0dbdf7bd5 100644
> --- a/langs/en/cgi-bin/en.pl
> +++ b/langs/en/cgi-bin/en.pl
> @@ -806,6 +806,9 @@
>  'dhcp dns update' => 'DNS Update',
>  'dhcp dns update algo' => 'Algorithm',
>  'dhcp dns update secret' => 'Secret',
> +'dhcp dynamic range overlap' => 'Dynamic range overlapped with ',
> +'dhcp fixed ip address' => ' Fixed IP Address(es)',
> +'dhcp fixed ip address in dynamic range' => 'Fixed IP Address in dynamic range is not allowed',
>  'dhcp fixed lease err1' => 'For a fix lease you have to enter the MAC address or the hostname, or you enter both.',
>  'dhcp fixed lease help1' => 'IP Addresses might be entered as FQDN',
>  'dhcp mode' => 'DHCP',
> --
> 2.30.1
>
>