Bug #13069
closed
Input validation for IPv6 addresses allows invalid address compression in some cases
Added by Marcos M over 2 years ago.
Updated over 2 years ago.
Plus Target Version:
22.05
Description
Tested on 22.05.a.20220412.0600.
There is no input validation for IPv6 addresses with multiple instances of the characters :: e.g. fc00::5::1. This prevents the firewall from booting, causes PHP crashes, and prevents the ruleset from being loaded correctly.
- Status changed from New to Feedback
- Priority changed from High to Normal
unable to reproduce - is_ipaddrv6('fc00::5::1') returns false
Same here, validation works fine in places I've tried it (e.g. alias content)
We will need a list of specific pages and settings which can trigger the failures. It's possible some fields lack full validation but it's definitely not lacking everywhere.
- Status changed from Feedback to Confirmed
Marcos sent me a different IPv6 string directly and that does validate when it should not, which I then used to check a few different ways and found there is a problem.
Net_IPv6::checkIPv6($ipaddr) checks the number of parts in the address and it validates if there are 8 parts, even if they are compressed with :: more than once, which is invalid. It's arguably unambiguous, because there are still exactly 8 parts, but it's still not valid.
var_dump(is_ipaddrv6('1:2:3:4::6::8'));
var_dump(is_ipaddrv6('1:2:3::5:6::8'));
var_dump(is_ipaddrv6('1:2::4:5:6::8'));
var_dump(is_ipaddrv6('1::3:4:5:6::8'));
var_dump(is_ipaddrv6('1:2::4::6::8'));
var_dump(is_ipaddrv6('1::3:4::6::8'));
var_dump(is_ipaddrv6('1::3::5:6::8'));
var_dump(is_ipaddrv6('1::3::5::7:8'));
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
It should outright reject anything with more than one occurrence of :: in the address.
- Status changed from Confirmed to In Progress
- Assignee set to Jim Pingle
- Target version set to 2.7.0
- Plus Target Version set to 22.05
- Status changed from In Progress to Pull Request Review
MR: https://gitlab.netgate.com/pfSense/pfSense/-/merge_requests/724
With the change in the MR, the results are as expected. Valid uses of single compression are OK, multiple are rejected.
var_dump(is_ipaddrv6('1:2:3:4:5:6:7:8'));
var_dump(is_ipaddrv6('1:2:3:4:5:6::8'));
var_dump(is_ipaddrv6('1:2:3:4::8'));
var_dump(is_ipaddrv6('1:2:3::5:6::8'));
var_dump(is_ipaddrv6('1:2::4:5:6::8'));
var_dump(is_ipaddrv6('1::3:4:5:6::8'));
var_dump(is_ipaddrv6('1:2::4::6::8'));
var_dump(is_ipaddrv6('1::3:4::6::8'));
var_dump(is_ipaddrv6('1::3::5:6::8'));
var_dump(is_ipaddrv6('1::3::5::7:8'));
bool(true)
bool(true)
bool(true)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
Tested with the IP that broke it previously in different places e.g. alias, interface, vip, freeradius. All worked (rejected with multiple instances of :: and accepted with the valid IP). Thanks!
- Status changed from Pull Request Review to Feedback
- % Done changed from 0 to 100
- Status changed from Feedback to Resolved
- Subject changed from Improve interface input validation for IPv6 to Input validation for IPv6 addresses allows invalid address compression in some cases
Updating subject for release notes.
Also available in: Atom
PDF
Updated by Viktor Gurov 
Updated by 
Updated by