395376 - url-classifier should check IP addresses as a single string

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Dave Camp (:dcamp)]

Right now the url-classifier treats a numeric IP as a hostname.  Checking 127.0.0.1 will search for 0.1 and 0.0.1 in the database.

Comment 1

[Dave Camp (:dcamp)]

Attachment: treat ip addresses as a single lookup key

Comment 2

[Tony Chang (Google)]

Comment on attachment 280062 [details] [diff] [review]
treat ip addresses as a single lookup key

Would the check pass for things like 1.2.3.4domain.com?  It looks like it would.  So maybe add a fifth token for a character and make sure only 4 tokens are read.

Alternately, you could be safe and do a two part check, a three part check and if it passes IsCanonicalizedIP, a four part check.

Comment 3

[Dave Camp (:dcamp)]

Attachment: v2

added a %c to the end of the sscanf.

Comment 4

[Dave Camp (:dcamp)]

Comment on attachment 281374 [details] [diff] [review]
v2

Asking for approval, this is needed to malware check ip addresses correctly.

Comment 5

[Boris Zbarsky [:bzbarsky]]

It looks like this will fail with IPv6.  It's also not clear to me that it handles username/password correctly, etc...

Why is this code hand-parsing URI strings anyway, instead of using nsIURI for things like "extract the host"?

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

my understanding was that IPv6 addresses don't need this because they don't contain dots

Comment 7

[Boris Zbarsky [:bzbarsky]]

Right, I see.  What about username/password?

Comment 8

[Dave Camp (:dcamp)]

This only operates on the output of the URI classifier's canonicalization process, which strips out the username/password.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Ah, excellent.  Sorry, I just tend to be suspicious when I see hand-parsing of URIs; we've had bugs pop up that way in the past.

Comment 10

[Dave Camp (:dcamp)]

Checking in toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp;
/cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp,v  <--  nsUrlClassifierDBService.cpp
new revision: 1.32; previous revision: 1.31
done