Page MenuHomeFreeBSD

pf: Improve ioctl() input validation
ClosedPublic

Authored by kp on Apr 15 2020, 6:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Oct 2 2024, 10:39 AM
Unknown Object (File)
Oct 2 2024, 10:36 AM
Unknown Object (File)
Sep 27 2024, 5:38 AM
Unknown Object (File)
Sep 26 2024, 8:49 AM
Unknown Object (File)
Sep 25 2024, 9:53 AM
Unknown Object (File)
Sep 25 2024, 9:53 AM
Unknown Object (File)
Sep 25 2024, 9:53 AM
Unknown Object (File)
Sep 25 2024, 9:49 AM

Details

Reviewers
None
Group Reviewers
network
Commits
rS360098: pf: Improve ioctl() input validation
Summary

Both DIOCCHANGEADDR and DIOCADDADDR take a struct pf_pooladdr from
userspace. They failed to validate the dyn pointer contained in its
struct pf_addr_wrap member structure.

This triggered assertion failures under fuzz testing in
pfi_dynaddr_setup(). Happily the dyn variable was overruled there, but
we should verify that it's set to NULL anyway.

Reported-by: syzbot+93e93150bc29f9b4b85f@syzkaller.appspotmail.com

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 30519
Build 28266: arc lint + arc unit

Event Timeline

Seems fine to me, although I'm wondering about having the kernel set it to NULL on entry vs requiring that it's NULL.

I think we want to check for NULL, as a robustness / future proofing measure.
Imagine that we eventually want to start using this variable for input (I don't know why, but for the sake of argument...), if we accepted random input there we could never tell the difference between an old binary (pre-using 'dyn' as input) passing garbage, and a new one passing a valid value.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 19 2020, 4:10 PM
This revision was automatically updated to reflect the committed changes.