Regression #13757
closed
Circular dependency issue in ``auth.inc``/``authgui.inc``
Added by Jim Pingle over 1 year ago.
Updated over 1 year ago.
Plus Target Version:
23.01
Release Notes:
Force Exclusion
Description
Some parts of auth.inc use a check for a function before doing some GUI-specific checks:
if (function_exists("display_error_form")) {
This function is defined in authgui.inc, but authgui.inc includes auth.inc before this function is defined.
I don't see how this could be working correctly unless something happened to include auth.inc twice.
The checks here used to work but perhaps something is behaving more appropriately in PHP 8.x which makes it (correctly) fail.
Looks like this may have broken in 746f30e3ce1ff39c226a73bf87c86dd370ef239c with the added includes changing the order in which things were initially loaded.
Moving the authgui.inc include up to the top of guiconfig.inc fixes the behavior, but it's still awkward.
- Status changed from New to Feedback
- % Done changed from 0 to 100
- Status changed from Feedback to In Progress
- % Done changed from 100 to 0
That fix attempt ended up not incomplete, it could break CSRF in certain cases.
Still experimenting and checking into alternative solutions.
Draft MR: https://gitlab.netgate.com/pfSense/pfSense/-/merge_requests/984
At the moment the least disruptive way to correct the behavior is to hardcode the CSRF timeout so it is not read from the configuration, which allows us to reorder includes so that authgui.inc can load auth.inc first.
I'm still looking into alternate solution that will allow reading in the CSRF timeout value from the configuration but it's a tricky chicken-and-egg scenario it ends up in because loading config.inc loads in numerous other includes, but we can't load authgui before initializing CSRF because it can output things that need CSRF protection.
- Status changed from In Progress to Feedback
- % Done changed from 0 to 100
- Status changed from Feedback to Resolved
Closing this for now as it appears to be working as expected given the current limitations for the moment.
I can trigger the DNS Rebind and/or HTTP_REFERER error by using an unknown hostname as expected depending on the context.
- Private changed from Yes to No
- Release Notes changed from Default to Force Exclusion
Also available in: Atom
PDF
Updated by