Custom client IP not validated

General Beta 5.0
1

Please follow the below template, it will help us to help you!

Please ensure that you are running the latest version of the beta code.
Run pihole -up to update to the latest, then verify that the problem still exists before reporting it.

Problem with Beta 5.0:
No IP validation when adding custom client IP

Debug Token:
None

Am i tripping or was there no validation before in

Group management --> Clients?

I looked all over github looking if this is something that maybe was temporalily
removed, but i can't seem to find any mention of this.

I'm confused because i know i've seen some validIP functions in some .php file somewhere before, and sure enough there is

in /var/www/html/admin/scripts/pi-hole/php/savesettings.php
line 14 validIP function

I can see how someone (i did, shhh... :stuck_out_tongue_closed_eyes:) insert localhost or any hostname to custom ip and not even realise that it's not working.

I swear it's used to be validated...?? :thinking:

2

No, I haven't gotten around adding validation here, this is on my Todo and, hopefully, I can get around it next week. I can assure you. it was never there.

It is not high priority as FTL is robust enough to just ignore anything that is not an IP address. Similarly to how you can throw any invalid domains or regex filters at it. Even containing UTF-8 characters or whatnot. It will just ignore incorrect user input.

1 Like
3

I must have thought about some other validation then and mix things up in my head, my bad.

I get it, keep up the great work!

4

I managed to catch up on this today. For this task, I had to come up with some quite complex regular expressions, any testing would be highly appreciated!

Check it out using:

pihole checkout web tweak/client_address_validation

Verify my regex here:

5

Nice!
Did some testing, ipv4 works well, but ipv6 can see some improvements, right now it accepts

1234:1234:1234:1234::1234:1234:1234:1234:1234:1234
if there is a :: it does not check the counts of elements.

6

Thanks for testing, I kind of have foreseen this, however, the complexity of the regex would rougly five-fold if I would include this. It should be safe to accept this (even if it will never happen). FTL will simply reject the address so no harm will be caused. The validator already got way beyond the point of what would have been needed :wink:

2 Likes
7

True, in any case, if people open up issues related to this then it can be implemented using php.
For me, as long as it prevents me from entering localhost :stuck_out_tongue: im happy.

1 Like
8
9

This PR has been merged, please go back onto release/v5.0 if you have tried this branch.