pfsense/pfsense — pull request #959 by stilez (2014-02-21 02:24:46+00:00)
Title: Tighten is_subnet() functions

The is_subnet(), is_subnetv4() and is_subnetv6() functions have significant issues in their coding logic.

Issues:

1) Functions use is_numeric(), so they validate invalid bitcount parts such as '1.1.1.1/6.5' or '::8000/94.7' as valid subnet strings

2) Validation explodes to a list of 2 variables rather than testing the whole string, so it's possible to craft a malicious string or mis-enter text such as "::8000/24/;\nARBITRARYTEXT;", or any valid subnet followed by a third '/' character followed by arbitrary text, which always validates as permitted data. In v2.x this allows arbitrary or malformed text placement into core config files such as pf.conf, f/w tables, and probably exec() strings, from the GUI (reported previously)

3) is_subnet() and is_subnetv6() return false for the valid subnet /0 as a CIDR, although technically uncommon this is allowed and valid as a subnet, and has an unambiguous meaning.

The existing code is easier to rewrite. It's a lot shorter too.

A single preg_match() separates valid subnet parts and also identifies which if either type of subnet the string could be, by exactly matching EITHER (7-15 char string containing only 0-9.) OR (2-39 char string containing only 0-9a-f:) followed by '/' and (1-3 numeric digits). Depending which of the first two matches is non-empty, it then checks the "ip" part" and "numeric" part are each valid, and returns. Efficient and robust.

NOTE: - these functions returned a True/false value ( is_subnet() or !is_subnet() ). As any positive number Boolean-evaluates to True, I have taken advantage of this to have "success" usefully return 4 or 6 (not just "true") to say the type of subnet matched. It's no extra effort or cost, transparent to any Boolean test., and allows a lot of efficiency and code simplification elsewhere, since calling code that needs to validate a subnet *and* test if it's IPv4 or IPv6, a common situation, can now do this reliably with a single is_subnet() call. Example use:

if (($sn = is_subnet($data)) {
if ($sn == 4) {
Immediately enter IPv4 handler code...
} else {
Immediately enter IPv6 handler code...
}
} else
Invalid data, enter error handler code...

I've used it in this PR to write is_subnetv4() and is_subnetv6() to be single line near-aliases of is_subnet(), that just check the return value is indeed 4 or 6.