Bug #14356
closed
URL scheme is not properly validated in some cases
100%
Description
Hello fellow pfSense Redmine community members can you please help?
If you generate an error inside of an Aliases specific when changing type "host" to "url(port)" the list of pink errors no longer displays, and this option now returns a PHP error.
Crash report begins. Anonymous machine information:
arm64
14.0-CURRENT
FreeBSD 14.0-CURRENT #0 plus-RELENG_23_01-n256037-6e914874a5e: Fri Feb 10 20:28:37 UTC 2023 root@freebsd:/var/jenkins/workspace/pfSense-Plus-snapshots-23_01-main/obj/aarch64/Z3hsU8Fs/var/jenkins/workspace/pfSense-Plus-snapshots-23_01-main/sources/Free
Crash report details:
PHP Errors:
[06-May-2023 15:43:48 US/Pacific] PHP Fatal error: Uncaught TypeError: implode(): Argument #1 ($pieces) must be of type array, string given in /usr/local/pfSense/include/www/alias-utils.inc:771
Stack trace:
#0 /usr/local/pfSense/include/www/alias-utils.inc(771): implode(' ', NULL)
#1 /usr/local/www/firewall_aliases_edit.php(157): saveAlias(Array, '19')
#2 {main}
thrown in /usr/local/pfSense/include/www/alias-utils.inc on line 771
[06-May-2023 15:43:51 US/Pacific] PHP Fatal error: Uncaught TypeError: implode(): Argument #1 ($pieces) must be of type array, string given in /usr/local/pfSense/include/www/alias-utils.inc:771
Stack trace:
#0 /usr/local/pfSense/include/www/alias-utils.inc(771): implode(' ', NULL)
#1 /usr/local/www/firewall_aliases_edit.php(157): saveAlias(Array, '19')
#2 {main}
thrown in /usr/local/pfSense/include/www/alias-utils.inc on line 771
No FreeBSD crash data found.
Please see attached
Files
| Screenshot 2023-05-06 at 3.50.40 PM.png (430 KB) Screenshot 2023-05-06 at 3.50.40 PM.png | change to url once saved error does not display pink errors however system crash occurs | ||
| Screenshot 2023-05-06 at 3.51.32 PM.png (263 KB) Screenshot 2023-05-06 at 3.51.32 PM.png | normally pink errors should have displayed like in past versions | ||
| Screenshot 2023-05-06 at 3.52.28 PM.png (319 KB) Screenshot 2023-05-06 at 3.52.28 PM.png | Error |
Updated by Kris Phillips over 1 year ago
Tested this in pfSense Plus 23.05 RC builds from May 6th. Created an Alias called "Test Alias" that was set to "Hosts" and added an entry for www.google.com. Saved and Applied.
Then re-edited the Alias, changed it to URL (Ports) with no other changes. The pink error box appeared without a crash. Seems this issue is either fixed in 23.05 or I'm missing a step in recreating this issue.
Updated by Jonathan Lee over 1 year ago
Exactly the steps I did to reproduce the issue. Thank you for checking.
Updated by Marcos M over 1 year ago
- Subject changed from 23.01 pfSense crash inside of Firewall / Aliases /Edit Dashboard. Pink error in GUI are no longer displaying for change overs from hosts to URLs to URL scheme is not properly validated in some cases
- Category changed from Aliases / Tables to Unknown
- Status changed from New to Pull Request Review
- Assignee set to Marcos M
- Affected Architecture All added
- Affected Architecture deleted (
SG-2100)
is_URL(), the URL scheme is required. If the subdomain www is specified, the function can return true even when the URL scheme is missing. This currently affects multiple areas:
- cert URI SAN
- alias with URL type
- captive portal preauthurl/redirurl/blockedmacsurl
- Check IP Service URL
- dhcp TFTP server / network bootfile
- OpenVPN ocspurl
Fix this by using native variable filters in PHP.
https://gitlab.netgate.com/pfSense/pfSense/-/merge_requests/1036
Updated by Jim Pingle over 1 year ago
Marcos M wrote in #note-3:
This currently affects multiple areas:
Affects how? They get a PHP error? Or they fail in some other way?
Updated by Marcos M over 1 year ago
- The OpenVPN input field is created with type="url" hence input validation is triggered before is_URL is called.
- The cert manager simply accepts the invalid url for the URI SAN without triggering a PHP or input validation error.
The reason for the PHP error on the URL-type ports alias is because this part evaluates to true:
https://github.com/pfsense/pfsense/blob/89803e07a756de18beab184b88ede15e6ba617ff/src/usr/local/pfSense/include/www/alias-utils.inc#L376
... which then leads to this evaluating to true and triggering the PHP error:
https://github.com/pfsense/pfsense/blob/89803e07a756de18beab184b88ede15e6ba617ff/src/usr/local/pfSense/include/www/alias-utils.inc#L744
Example:
var_dump(is_URL('http://google.com')); # true
var_dump(is_URL('http://www.google.com')); # true
var_dump(is_URL('google.com')); # false
var_dump(is_URL('www.google.com')); # true - should be false
Updated by Jim Pingle over 1 year ago
- Target version set to 2.7.0
- Plus Target Version set to 23.05
I just don't want to get caught in a situation where something big relies on the current behavior that would be a pain to adjust. If everything, including packages, should already be using full URLs with a scheme, then it should be OK to change it over.
We've had some issues in the past with that function and changes to it ended up having unintended fallout, but maybe that's all been addressed since then.
Updated by Jim Pingle over 1 year ago
- Plus Target Version changed from 23.05 to 23.09
Updated by Jonathan Lee over 1 year ago
Thank you for looking at this, I thought I should share it as it could possibly be used with a zero day if it is not corrected. Thank you for reviewing this. Have a good day everyone.
Updated by Marcos M over 1 year ago
- Status changed from Pull Request Review to Feedback
- % Done changed from 0 to 100
Applied in changeset 7a14ab5dd8b35db9da7163ab97e9d2f7452f8cfb.
Updated by Jim Pingle over 1 year ago
- Category changed from Unknown to PHP Interpreter
Updating for release notes.