pfsense/pfsense — pull request #961 by stilez (2014-02-21 03:13:41+00:00)
Title: Tighten is_port()

is_port() should, from its name, validate a single port. A separate function is_portrange(), exists and validates colon-separated port ranges such as 1000:1999. However this function has severe issues. This PR fixes three of the main issues - the rest may need coming back to later:

1) is_port() validates any ':' separated list of ports as "a port", even if the calling routine wants to validate that the data represents one single port. This can cause errors in $config[], pf or the calling code.

2) is_port() also treats ':' ambiguously. If '1:6' means a port range, what does '1:6:9' mean, which is also validated by is_port() if entered in the GUI at many points? (Obviously malformed but validates)

3) is_port() returns false for port 0. While undefined, port 0 exists and has well-known security implications and users must be able to specify it where appropriate (if the OS or s/w blocks it that's a different issue, but the user needs to be able to refer to it or specify packet capture or logging rules on it!)

Eg:
http://www.pcworld.com/article/2061080/spike-in-traffic-with-tcp-source-port-zero-has-some-researchers-worried.html
https://tools.cisco.com/security/center/viewAlert.x?alertId=19935
http://grc.com/port_0.htm

I've searched the codebase for calls to is_port(). There are only about ~30 or so instances, and none of therm seem to obviously expect to test anything except one, single port. So I think the explode/foreach part is an error, it makes sense in is_portrange() but not in is_port(). I've cut is_port right down to simply validate one numeric port (0-65535) or hard-coded definition, which seems to be what this function is actually meant to be doing.