security: open redirect hardening (main)#771
security: open redirect hardening (main)#771somethingwithproof wants to merge 1 commit intoCacti:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Backports open-redirect hardening in the thold plugin by sanitizing redirects that use HTTP_REFERER in thold.php.
Changes:
- Replaces direct
$_SERVER['HTTP_REFERER']redirects withvalidate_redirect_url()for theenableanddisableactions. - Simplifies the referer/default selection logic into a single validated assignment.
| $return_to = 'thold.php'; | ||
| } | ||
|
|
||
| $return_to = validate_redirect_url(isset($_SERVER['HTTP_REFERER']) ? $_SERVER['HTTP_REFERER'] : 'thold.php'); |
There was a problem hiding this comment.
validate_redirect_url() is not defined anywhere in this plugin repo, so this call will fatally error unless the hosting Cacti core version provides it. To avoid breaking installs on supported versions, either (a) bump/enforce the minimum Cacti version that includes validate_redirect_url(), or (b) guard with function_exists() and fall back to a local redirect-validator implementation.
| $return_to = 'thold.php'; | ||
| } | ||
|
|
||
| $return_to = validate_redirect_url(isset($_SERVER['HTTP_REFERER']) ? $_SERVER['HTTP_REFERER'] : 'thold.php'); |
There was a problem hiding this comment.
Same concern here: calling validate_redirect_url() without ensuring it exists will cause a fatal error on Cacti versions that don't ship that helper. Consider enforcing the required Cacti version in setup.php/README or adding a function_exists() fallback.
| $return_to = validate_redirect_url(isset($_SERVER['HTTP_REFERER']) ? $_SERVER['HTTP_REFERER'] : 'thold.php'); | ||
| header('Location: ' . $return_to . (strpos($return_to, '?') !== false ? '&':'?') . 'header=false'); |
There was a problem hiding this comment.
This change hardens redirects based on HTTP_REFERER, but autocreate still redirects to $_SESSION['data_return'] which is populated from HTTP_REFERER using only a regex match (e.g., an external URL ending in data_sources.php would still be accepted). For consistent open-redirect protection, validate/sanitize the referer when setting data_return and/or before issuing the Location: header in the autocreate branch.
Backports open redirect fixes by applying validate_redirect_url() to HTTP_REFERER redirects in thold.php.