Bug 260958 - pfctl: expand_rule: strlcpy
Summary: pfctl: expand_rule: strlcpy
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 13.0-STABLE
Hardware: Any Any
: --- Affects Many People
Assignee: Kristof Provost
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-01-05 18:19 UTC by Thomas Steen Rasmussen / Tykling
Modified: 2024-11-25 04:18 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Steen Rasmussen / Tykling 2022-01-05 18:19:33 UTC
Hello :)

A fun bug where a long-ish ipv6 address used in a reply-to in a pf ruleset results in pfctl being unable to parse the ruleset.

The issue boiled down to a one line ruleset, one with a 16 byte address (which fails) and the other with a 15 byte address (which works).

[tykling@nuc1 ~]$ cat trigger 
pass in reply-to { 2001:DB8:1234::5 }
[tykling@nuc1 ~]$ pfctl -nf trigger 
pfctl: expand_rule: strlcpy
[tykling@nuc1 ~]$ echo $?
1
[tykling@nuc1 ~]$ cat notrigger 
pass in reply-to { 2001:DB8:1234:: }
[tykling@nuc1 ~]$ pfctl -nf notrigger 
[tykling@nuc1 ~]$ echo $?
0
[tykling@nuc1 ~]$ uname -a
FreeBSD nuc1.servers.bornhack.org 13.0-STABLE FreeBSD 13.0-STABLE #1 stable/13-d208638c5: Wed Jan  5 13:32:08 UTC 2022     root@nuc1.servers.bornhack.org:/usr/obj/usr/src/amd64.amd64/sys/GENERIC  amd64
[tykling@nuc1 ~]$ 

We first observed this on 12.2-STABLE a while back but I didn't get around to reporting it until now, so I've just confirmed it is still an issue on a fresh 13-STABLE.

Thanks! :)
Comment 1 Kristof Provost freebsd_committer freebsd_triage 2022-01-05 21:18:45 UTC
I can reproduce the error, but my current thinking is that this is a misconfiguration rather than a bug in pf. (Although arguably the error message could be better).

Note that reply-to expects to be followed by a routehost ('     routehost      = "(" interface-name [ address [ "/" mask-bits ] ] ")"', which is supposed to be an interface-name and optionally an address.

The pfctl parser code puts the string it finds after route-to in an ifname field, which is IFNAMSIZ bytes long, so the IPv6 address you provide doesn't fit and that's what produces the error message.
Comment 2 Thomas Steen Rasmussen / Tykling 2022-01-05 21:22:22 UTC
Hello,

Yes, I should have mentioned that, of course.

It isn't a valid config, but normally we would get a message from pfctl with the line number and a "syntax error" message, so this seemed like something that should be fixed :)

Thanks!
Comment 3 commit-hook freebsd_committer freebsd_triage 2022-01-27 07:51:20 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e68de6694381748b7578703b22580c0f17780b32

commit e68de6694381748b7578703b22580c0f17780b32
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2022-01-05 20:31:02 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2022-01-27 06:36:26 +0000

    pfctl: improve error reporting for routehost

    If an invalid (i.e. overly long) interface name is specified error out
    immediately, rather than in expand_rule() so we point at the incorrect
    line.

    PR:             260958
    MFC after:      3 weeks
    Differential Revision:  https://reviews.freebsd.org/D34008

 sbin/pfctl/parse.y | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
Comment 4 commit-hook freebsd_committer freebsd_triage 2022-02-18 10:47:04 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b5f6f687a2eea520d93f2c1ca4e04efd7c2e367f

commit b5f6f687a2eea520d93f2c1ca4e04efd7c2e367f
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2022-01-05 20:31:02 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2022-02-18 10:14:58 +0000

    pfctl: improve error reporting for routehost

    If an invalid (i.e. overly long) interface name is specified error out
    immediately, rather than in expand_rule() so we point at the incorrect
    line.

    PR:             260958
    MFC after:      3 weeks
    Differential Revision:  https://reviews.freebsd.org/D34008

    (cherry picked from commit e68de6694381748b7578703b22580c0f17780b32)

 sbin/pfctl/parse.y | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2022-02-18 10:47:06 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=8ac3a178534344d8b3b0295b831cab763d466c19

commit 8ac3a178534344d8b3b0295b831cab763d466c19
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2022-01-05 20:31:02 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2022-02-18 10:15:31 +0000

    pfctl: improve error reporting for routehost

    If an invalid (i.e. overly long) interface name is specified error out
    immediately, rather than in expand_rule() so we point at the incorrect
    line.

    PR:             260958
    MFC after:      3 weeks
    Differential Revision:  https://reviews.freebsd.org/D34008

    (cherry picked from commit e68de6694381748b7578703b22580c0f17780b32)

 sbin/pfctl/parse.y | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
Comment 6 Mark Linimon freebsd_committer freebsd_triage 2024-11-25 04:18:36 UTC
^Triage: committed and MFCed back in 2022.