pfsense/pfsense — pull request #960 by stilez (2014-02-21 02:49:16+00:00)
Title: Tighten is_validaliasname()

is_validaliasname() treats "empty string" as a valid alias name, it probably shouldn't.

I suspect it also should not allow purely numeric names ('53'), or pure underscore ('_'), or reserved port names ('tcp', 'http'), as valid alias names for other things. Too much risk of issue/ambiguity which isn't helpful in a router/security device, and no obvious upside to it.

Example: - Suppose I define '53' as an alias for '54' or 'tcp' as an alias for 'ssh' or 22 on some firewall..... which I can because is_validaliasname() will validate both? It's a subtle change, hard to spot as rules will look correct, easy to cause problems or wilfully misuse.)

"Pure numeric" aliases are also worryingly ambiguous since many other things that can be entered or appear on the GUI, such as port numbers, can be entered or will show as pure numeric or an alias, so there's scope for ambiguity which it means. (is '53' the port 53, or the alias 53 which I've redefined to mean 1053?)

This PR generally tightens the logic and regex expressions (shorter syntax, no '?:'s or '?' groups), and slightly but sanely limits valid alias names to remove ambiguity - specifically it bars 4 kinds of string (empty string, pure numeric, pure underscore, and PHP reserved names that are already hard-coded aliases for specific protocols/services) from being valid alias names any more, as a result.

(I think this is sane, if you agree!)