Project

General

Profile

Actions

Bug #15263

closed

PHP error display formatting issues

Added by Jim Pingle 10 months ago. Updated 8 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
PHP Interpreter
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
24.03
Release Notes:
Default
Affected Version:
Affected Architecture:

Description

There are multiple issues with the formatting of PHP errors in the GUI, including:

  • Error/stack trace is printed twice
  • Error message is plain text (not HTML) but is getting formatted like HTML
  • Function arguments are printed in the stack trace which may contain user input

The first item is due to display_errors being on in php.ini but there is no need to display errors directly from PHP, they are printed in source:src/etc/inc/config.lib.inc#L1168 by pfSense_clear_globals() -- Changing display_errors to off is sufficient to address this.

The second point can be handled by wrapping the error output in tags or some variation thereof, plus encoding the error message itself. The relevant code where the errors are printed is in in pfSense_clear_globals() in source:src/etc/inc/config.lib.inc#L1168 (or close to there).

The second and third points combined have a potential to lead to an XSS if the user can influence the arguments displayed in the stack trace. So not only should we take more care when printing the errors (encode via htmlentities() or htmlspecialchars() in pfSense_clear_globals() in source:src/etc/inc/config.lib.inc#L1168) but on release versions we should suppress the function arguments in stack traces entirely (zend.exception_ignore_args=1 in php.ini)

I have a tested and working patch ready to commit.

Actions #1

Updated by Jim Pingle 10 months ago

  • Status changed from In Progress to Feedback
  • % Done changed from 0 to 100
Actions #2

Updated by Jim Pingle 10 months ago

  • Status changed from Feedback to In Progress

This change seems to have broken the "error locator" on diag_command.php. If you deliberately try to run broken code there it comes up blank.

Actions #3

Updated by Jim Pingle 10 months ago

  • Status changed from In Progress to Feedback
Actions #4

Updated by Jim Pingle 10 months ago

  • Status changed from Feedback to Resolved

Everything appears to be working properly on the latest snapshot.

Actions #5

Updated by Jim Pingle 8 months ago

  • Private changed from Yes to No
Actions

Also available in: Atom PDF