638559 - don't record internal IP addresses

Status: VERIFIED FIXED

(Input Graveyard :: Backend, enhancement)

Description

[Kevin Brosnan [Ex-Mozilla]]

On input I see some URLs that point to internal networks. The 10.0.0.0/8, 172.16.0.0/14 and 192.168.0.0/16 blocks are set aside for IPv4

Comment 1

[Fred Wenzel [:wenzel]]

Thanks, of course you are right. We can add that, though this brings us into the vague vicinity of the "make sure a URL is useful" problem, which we've been trying to avoid (we don't check that it's not a 404, for example). Although since this can be checked offline, I think it can be added. As always, patches welcome :)

Comment 2

[Fred Wenzel [:wenzel]]

Marking as good first bug: This just needs to become another standard form validity check on the URL field. URLs are parsed already, so if the hostname is an IPv4 address, we could further parse that and ensure it is not a private IP, throw an error otherwise.

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

Should we:
1) accept the input and silently drop the private ip blocks? 
2) Accept the input and group it as a problem with a private ip block? 
3) Return an error to the user and ask for a good url or no url to be submitted?
4) other?

Comment 4

[Fred Wenzel [:wenzel]]

I don't like 1) because it silently pretends their input was okay, but in fact we drop it.

2) is an option, which would address an odd (theoretical) class of errors that only occurs on local-IP websites. But building a webapp for the special case is not great.

3) That's the question here. Either we don't care (since we don't care about nonexistant domain names either, just blatantly *invalid* onces), or we do care, in which case we should return an error message indicating that the IP is private and thus irreproducible for us.

Comment 5

[tofumatt [:tofumatt]]

Added a validator that disallows private IPs in URLs and complains about them.

r? on my IPv4 regex: https://github.com/tofumatt/reporter/commit/84606f1f83e1515891f403ae467264cb52255df0

Comment 6

[tofumatt [:tofumatt]]

Updated this a bit (regex is now cAsE iNsEnSiTiVe). https://github.com/fwenzel/reporter/commit/8204c7ddca508d8732687f5f8e3d1d2754ab57c8

Comment 7

[Matt Brandt [:mbrandt]]

QA verifie internal IP addresses are rejected. Tested with several blocks of addresses within 10.0.0.0/8, 172.16.0.0/14 and 192.168.0.0/16 ranges.

Comment 8

[Matt Brandt [:mbrandt]]

Attachment: screen shot

screen grab of the new message.

Comment 9

[Matt Brandt [:mbrandt]]

bug 662801 filed to add localhost to the IP address block list.