Bug #14702: ``ctype_digit()`` returns unexpected result for values <= ``255`` which can break some validation functions/usages - pfSense - pfSense bugtracker

Actions

Copy link

Bug #14702

closed

``ctype_digit()`` returns unexpected result for values <= ``255`` which can break some validation functions/usages

Added by John Uplink over 1 year ago. Updated about 1 year ago.

Status:

Resolved

Priority:

Normal

Assignee:

Jim Pingle

Category:

Rules / NAT

Target version:

2.7.1

Start date:

Due date:

% Done:

100%

Estimated time:

Plus Target Version:

23.09

Release Notes:

Default

Affected Version:

Affected Architecture:

Description

Hello pfSense,

I've noticed that when you create a NAT rule with a port range starting with 20 (e.g. 20-21 or 20-28, etc.) it will erroneously create a corresponding Firewall rule that allows all ports. Please see steps to reproduce and screenshots below for details.

NOTE: This was working in pfsense 2.6 CE, but broke when I upgraded to 23.05.1 plus edition. I can also confirm this bug on another device runing 23.05.01 plus that was previously upgraded from 2.7 CE.

Steps to reproduce:
1. Create a new NAT rule with port range 20 to 21 and target port of 20 (see 1st screenshot below)
2. Ensure "add associated filter rule" is selected at the bottom
3. Save rule and navigate to Firewall rules
4. Expected: A new Firewall rule is generated with a destination port range of 20-21
5. Actual: A new Firewall rule is generated with a destination port of ALL ports! (see 2nd screenshot below)

Files

Download all files

NAT_Rule_Creation.jpg (315 KB) NAT_Rule_Creation.jpg		John Uplink, 08/21/2023 06:38 AM
Firewall_Bug.jpg (171 KB) Firewall_Bug.jpg		John Uplink, 08/21/2023 06:39 AM

Actions

Copy link

Updated by Jim Pingle over 1 year ago

Project changed from pfSense Plus to pfSense
Subject changed from NAT Rules Created With A Port Range Starting With Port 20 Generates Incorrect Firewall Rules to Associated firewall rule for a NAT port forward range involving certain low port ranges does not contain proper port restrictions
Category changed from Rules / NAT to Rules / NAT
Status changed from New to Confirmed
Assignee set to Jim Pingle
Target version set to 2.8.0
Affected Plus Version deleted (~~23.05.1~~)
Plus Target Version set to 23.09

This isn't specific to FTP, it happens for a few different ranges I tried (10-11, 20-21, 100-101, etc.) though it doesn't seem to happen for higher ranges (At least >400).

The values are being passed but something in the port calculation is off, like maybe somehow it's failing an is_port() test unexpectedly.

Actions

Copy link

Updated by Jim Pingle over 1 year ago

The problem is with the ctype_digit() test used in is_port():

https://www.php.net/manual/en/function.ctype-digit.php

Note:
If an int between -128 and 255 inclusive is provided, it is interpreted as the ASCII value of a single character (negative values have 256 added in order to allow characters in the Extended ASCII range). Any other integer is interpreted as a string containing the decimal digits of the integer.

Warning
As of PHP 8.1.0, passing a non-string argument is deprecated. In the future, the argument will be interpreted as a string instead of an ASCII codepoint. Depending on the intended behavior, the argument should either be cast to string or an explicit call to chr() should be made.

Actions

Copy link

Updated by Jim Pingle over 1 year ago

Subject changed from Associated firewall rule for a NAT port forward range involving certain low port ranges does not contain proper port restrictions to ``ctype_digit()`` returns unexpected result for values <= ``255`` which can break some validation functions/usages
Status changed from Confirmed to In Progress

Looks like this could also break things in a few other places since we use that function ~10 times in various files.

Adding a strval() around the parameter seems to make it work as we want, so ctype_digit(strval($whatever)).

Fix coming shortly.

Actions

Copy link