Project

General

Profile

Actions

Bug #1043

closed

Inadequate input validation on limiters with floating rules

Added by Chris Buechler about 11 years ago. Updated almost 11 years ago.

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

100%

Estimated time:
Plus Target Version:
Release Notes:
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.

Actions #1

Updated by Ermal Luçi about 11 years ago

  • Status changed from New to Feedback
  • % Done changed from 0 to 100
Actions #2

Updated by Alexander Kalashnikov almost 11 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".

Actions #3

Updated by Chris Buechler almost 11 years ago

  • Status changed from Feedback to New
Actions #4

Updated by Josh Stompro almost 11 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

Actions #5

Updated by Josh Stompro almost 11 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

Actions #6

Updated by Chris Buechler almost 11 years ago

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

Actions #7

Updated by Ermal Luçi almost 11 years ago

  • Status changed from New to Feedback

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

Actions #8

Updated by Alexander Kalashnikov almost 11 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"))

Actions #9

Updated by Alexander Kalashnikov almost 11 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"))

Actions #10

Updated by Chris Buechler almost 11 years ago

  • Status changed from Feedback to New
Actions #11

Updated by Ermal Luçi almost 11 years ago

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

Actions #12

Updated by Alexander Kalashnikov almost 11 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'?

Actions #13

Updated by Alexander Kalashnikov almost 11 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"))

Actions #14

Updated by Ermal Luçi almost 11 years ago

  • Status changed from New to Feedback

Yeah, thank you for catching that wrong check.

Actions #15

Updated by Alexander Kalashnikov almost 11 years ago

No problem.

It's working now.

Thank you.

Actions #16

Updated by Chris Buechler almost 11 years ago

  • Status changed from Feedback to Resolved

thanks

Actions

Also available in: Atom PDF