pfSense

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 this is allowed and valid as a subnet (it may have unambiguous and useful meaning in a router or security context).

The existing code is easier to rewrite than fix. It's a lot shorter too. A single preg_match() does all the separating of valid subnet parts and 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 exits. Efficient and robust.

NOTE: - these functions returned a True/false value ( is_subnet() or !is_subnet() ). As any positive number evaluates in a Boolean sense to True, I have taken advantage of this to have "success" return 4 or 6 (not just "true") to say the type of subnet matched. It's no extra effort or cost, and turns out to allow 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:

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

I've used it here 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.