Project

General

Profile

Bug #1043

Inadequate input validation on limiters with floating rules

Added by Chris Buechler over 8 years ago. Updated over 8 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
Rules/NAT
Target version:
Start date:
11/25/2010
Due date:
% Done:

100%

Estimated time:
Affected Version:
2.0
Affected Architecture:

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.

Associated revisions

Revision 02d7e4a4 (diff)
Added by Ermal Luçi over 8 years ago

Resolves #1043. Do not allow limiters in floating rules without direction. It is invalid practice and while the backend skips it the user should be warned.

Revision 622bd5e7 (diff)
Added by Ermal Luçi over 8 years ago

Ticket #1043. Check for '' and not for 'default' since this is the default value of the select.

History

#1 Updated by Ermal Luçi over 8 years ago

  • Status changed from New to Feedback
  • % Done changed from 0 to 100

#2 Updated by Alexander Kalashnikov over 8 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".

#3 Updated by Chris Buechler over 8 years ago

  • Status changed from Feedback to New

#4 Updated by Josh Stompro over 8 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

#5 Updated by Josh Stompro over 8 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

#6 Updated by Chris Buechler over 8 years ago

this particular issue isn't a problem, but yeah the fix broke other things.

#7 Updated by Ermal Luçi over 8 years ago

  • Status changed from New to Feedback

I fixed even the regression caused by fixing the limiters.
Test it with latest snapshots.

#8 Updated by Alexander Kalashnikov over 8 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"))

#9 Updated by Alexander Kalashnikov over 8 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"))

#10 Updated by Chris Buechler over 8 years ago

  • Status changed from Feedback to New

#11 Updated by Ermal Luçi over 8 years ago

Hah it seems you cannot read code!
That code is correct!

#12 Updated by Alexander Kalashnikov over 8 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'?

#13 Updated by Alexander Kalashnikov over 8 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"))

#14 Updated by Ermal Luçi over 8 years ago

  • Status changed from New to Feedback

Yeah, thank you for catching that wrong check.

#15 Updated by Alexander Kalashnikov over 8 years ago

No problem.

It's working now.

Thank you.

#16 Updated by Chris Buechler over 8 years ago

  • Status changed from Feedback to Resolved

thanks

Also available in: Atom PDF