Bug #1043
closed
Inadequate input validation on limiters with floating rules
100%
Description
With floating rules, it's possible to create an invalid ruleset by specifying a limiter on a rule without a direction. pf just skips the affected rule(s), showing "dummynet cannot be specified without a direction". Need input validation to ensure direction is specified on any rule using limiters.
Updated by Ermal Luçi almost 14 years ago
- Status changed from New to Feedback
- % Done changed from 0 to 100
Applied in changeset 02d7e4a43b6c6e1f8345c7561394305185905b33.
Updated by Alexander Kalashnikov almost 14 years ago
This fix led to that every rule in Floating tab MUST contain a direction.
So now it's unable to create there a rule with direction set to "Any".
Updated by Chris Buechler almost 14 years ago
- Status changed from Feedback to New
Updated by Josh Stompro almost 14 years ago
Tested on 2.0-beta4 (i386) Dec 10 02:17:09:EST 2010
When I tried to add a limiter (In/Out, which is not a very descriptive label if you don't know what you are looking for) to a floating rule with direction set to any, I get the error "You can not use limiters in floating rules without choosing a direction"
So I think this is resolved.
Josh
Updated by Josh Stompro almost 14 years ago
Whoops, didn't see the comment by Alexander when I posted.
I can confirm the bug he reported, setting direction to any, and not touching the limiters results in the same error.
Josh
Updated by Chris Buechler almost 14 years ago
this particular issue isn't a problem, but yeah the fix broke other things.
Updated by Ermal Luçi almost 14 years ago
- Status changed from New to Feedback
I fixed even the regression caused by fixing the limiters.
Test it with latest snapshots.
Updated by Alexander Kalashnikov almost 14 years ago
Unfortunatelly it's not fixed.
Problem is in program logic:
http://redmine.pfsense.org/projects/pfsense/repository/revisions/6735d0929eee41ef1cf2f253fa2a50740c066660/diff/usr/local/www/firewall_rules_edit.php
if (isset($_POST['floating']) && $_POST['pdnpipe'] != "none" && (empty($_POST['direction']) || $_POST['direction'] "any"))
Replace OR to AND.
"If it is floating rule and it is not a 'none' limiter and a direction is "any"
Something like this:
if (isset($_POST['floating']) && $_POST['pdnpipe'] != "none" && (empty($_POST['direction']) && $_POST['direction'] "any"))
Updated by Alexander Kalashnikov almost 14 years ago
The same needs to be done in:
&& $_POST['gateway'] != "default" && (empty($_POST['direction']) || $_POST['direction'] "any"))
change to
&& $_POST['gateway'] != "default" && (empty($_POST['direction']) && $_POST['direction'] "any"))
Updated by Chris Buechler almost 14 years ago
- Status changed from Feedback to New
Updated by Ermal Luçi almost 14 years ago
Hah it seems you cannot read code!
That code is correct!
Updated by Alexander Kalashnikov almost 14 years ago
Sure I can read code and any text since I've read your response and writing an answer here.
I'm sorry for that I've misguided you when I've missed double braces in if-clause.
Anyway, if the code is correct, could you, please, provide me an instruction on how to create a Floating rule without queue\gateway and with direction set to 'any'?
Updated by Alexander Kalashnikov almost 14 years ago
It seems like the issue is still present but only for gateways check since the $_POST['gateway'] contains an empty string when default gateway is used.
I've checked it in firefox, ie(9) and opera.
So the code should be something like:
$_POST['gateway'] != "" && (empty($_POST['direction']) || $_POST['direction'] == "any"))
Updated by Ermal Luçi almost 14 years ago
- Status changed from New to Feedback
Yeah, thank you for catching that wrong check.
Updated by Alexander Kalashnikov almost 14 years ago
No problem.
It's working now.
Thank you.