Bug #6272
closedWrong numbers in state column of /firewall_rules.php
100%
Description
Hello,
I have a firewall rule which have weird numbers on the state column, not sure why. Attached is a screenshot.
It's a simple IPv4 TCP NAT rule.
Tell me if you need more informations
Thanks
Files
Updated by Chris Buechler over 8 years ago
what does the output of 'pfctl -vvsr' show?
Updated by Jo S over 8 years ago
Here is the output for this rule:
@78(1422127143) pass in quick on em0 reply-to (em0 192.168.10.1) inet proto tcp from any to <Synology:1> port = 16881 flags S/SA keep state label "USER_RULE: NAT Bittorent Synology TCP"
[ Evaluations: 452882 Packets: 3767666 Bytes: 2290434228 States: 2 ]
[ Inserted: pid 54729 State Creations: 8 ]
Updated by Phillip Davis over 8 years ago
That is a wraparound of a 32-bit signed integer for 2,290,434,228 bytes.
@Jo S - is this on a 32-bit system?
Updated by Phillip Davis over 8 years ago
I tried adding integers on a 32-bit system until they got too big - they happily change into float, and format_bytes() does a reasonable job reporting as GiB or TiB.
It seems that the negative number is coming directly from something in pfSense_get_pf_rules() - which is the code that parses the rule(s) and returns an array of numbers that includes the byte count. I am guessing that pfSense_get_pf_rules() on 32-bit systems is using a 32-bit signed int internally and getting the wrap-around to a negative number. The negative number gets pass through all the PHP and formatted like in the screen shot.
I will leave it to someone who knows where it is to look into pfSense_get_pf_rules() source code and sort something out - maybe there is a "long long int" available on 32-bit systems that will cope, or maybe it can just parse the long number as a string of digits, and hand the string to PHP, which will happily make it an int or round it into a float if it is too big...
Updated by Jo S over 8 years ago
Seems to be there:
https://github.com/pfsense/FreeBSD-ports/blob/devel/devel/php56-pfSense-module/files/pfSense.c#L2342
add_assoc_long(array, "bytes", (long)(pr.rule.bytes[0] + pr.rule.bytes[1]));
May be there should be some check against LONG_MAX and convert to string ?
Updated by Phillip Davis over 8 years ago
A bunch of other places cast various numbers to (unsigned long long) - that might be all that is needed, and perhaps casting each number to (unsigned long long) before adding them, to make sure they add OK:
add_assoc_long(array, "bytes", (unsigned long long)((unsigned long long)pr.rule.bytes[0] + (unsigned long long)pr.rule.bytes[1]));
Maybe something like that?
And whatever works, also do it to the other add_assoc_long() calls in that function, in case any of those overflow one day.
I don't have any sort of build environment for that sort of thing to be able to mess about with it.
Updated by Luiz Souza almost 8 years ago
- Status changed from New to Resolved
- % Done changed from 0 to 100
Fixed on 2.4 and RELENG_2_3.
pfSense_get_pf_states() now return the packet counters as doubles.
Updated by Jo S almost 8 years ago
Hi, by "RELENG_2_3" do you mean this should be already fixed in current stable 2.3.2-RELEASE-p1 ? Because the problem is still there in this version.
Updated by Phillip Davis almost 8 years ago
RELENG_2_3 is the development path towards (a possible) 2.3.3. It should therefore be fixed in recent builds of 2.3.3-DEVELOPMENT