Hello Bob,

Thank you for submitting your patch.

> On 12 Dec 2018, at 22:48, Bob Brewer <ipfire-devel(a)grantura.co.uk> wrote:
> 
> I am porting the old ipcop addon 'Banish' to IPFire and during testing have 
> found a problem in general-functions.pl which causes validfqdn to return 1 
> when testing valid and invalid ip addresses when it should return 0. 

What does the add-on do? I could not find an old version for IPCop on the Internet…

> As this is not a problem with IPCop 2 a comparison of the validfqdn section 
> in IPFire's general-functions.pl shows a missing segment that checks the TLD 
> can only be a-z or A-Z.

What requires this change?

I do not know of any ASCII TLDs that have numbers, but there is no reason that they can’t in the future. Furthermore, we have some non-ASCII TLDs which will have to be encoded into ASCII using the puny-codes. That will result in something like this:

XN--FHBEI
XN--FIQ228C5HS
XN--FIQ64B
XN--FIQS8S
XN—FIQZ9S

This is just a couple of random TLDs I picked from here:

  http://data.iana.org/TLD/tlds-alpha-by-domain.txt

I assume that those will no longer be usable after your patch. Can you confirm that?

Best,
-Michael

> Applying the patch below to general-functions.pl corrects the problem with 
> my Banish port and I haven't found any problems affecting IPFire's 
> operation.
> 
> Regards
> 
> Rob
> 
> --- /tmp/general-functions.pl   2018-09-19 10:32:37.000000000 +0100
> +++ /tmp/general-functions.pl.new       2018-12-12 22:13:37.394653609 +0000
> @@ -666,9 +666,13 @@
> }
> 
> sub validfqdn
> +# modified to add addition test to confirm TL is only a-z or A-Z
> +# as per ipcop rwb 12/12/18
> +
> {
>        my $part;
> -
> +        my $tld;
> +     
>        # Checks a fully qualified domain name against RFC1035
>         my $fqdn = $_[0];
>        my @parts = split (/\./, $fqdn);        # Split hostname at the '.'
> @@ -689,7 +693,14 @@
>                # Last character can only be a letter or a digit
>                if (substr ($part, -1, 1) !~ /^[a-zA-Z0-9]*$/) {
>                        return 0;}
> -       }
> +           # Store for additional check on TLD
> +           $tld = $part;
> +        } 
> +
> +        # TLD valid characters are a-z, A-Z
> +        if ($tld !~ /^[a-zA-Z]*$/) {
> +        return 0;
> +        }
>        return 1;
> }
>