Bug #15738
closedxml parsing: whitespace causes errors.
0%
Description
Overview
Modifying the insignificant whitespace in config.xml can lead to an error that may brick a pfSense firewall.
*Steps to reproduce: *
1. Start with a clean pfSense
2. Modify the file /etc/inc/xmlparse.inc and replace this line of code in the function startElement($parser, $name, $attrs);
$ptr = &$ptr[$path];
with this block:
try {
$cptr = $ptr;
$ptr = &$ptr[$path];
} catch(TypeError $e) {
if(is_array($curpath)) {
$fullPath = implode(".", $curpath);
} else {
$fullPath = $curpath;
}
$idx = is_array($cptr) ? count($cptr) + 1 : 0;
$e2 = new TypeError($e->getMessage() . " PATH: {$fullPath}, INDEX: {$idx}");
throw $e2;
}
I do this to get a slightly better error message.
3. Create one additional user. (Values don't matter)
4. Then, backup your configuration.
5. Go into the config.xml output of this backup with any text editor. Add four spaces between the users (directly behind </user>), i.e. the following change:
09 09 3C 2F 75 73 65 72 3E 0A 09 09 3C 75 73 65 72 3E
to:
20 20 20 20 20 20 20 20 3C 2F 75 73 65 72 3E 20 20 20 20 0A 20 20 20 20 20 20 20 20 3C 75 73 65 72 3E
Note: Your bugtracker website may mangle the [semantically important] whitespace so I formatted the change in hexadecimal.
6. Restore the configuration.
Exprected Result
No change in configuration.
Actual result
The configuration page is replaced with this PHP fatal error.
Fatal error: Uncaught TypeError: Cannot access offset of type string on string PATH: pfsense.system.user, INDEX: 0 in /etc/inc/xmlparse.inc:83 Stack trace: #0 [internal function]: startElement(Object(XMLParser), 'USER', Array) #1 /etc/inc/xmlparse.inc(213): xml_parse(Object(XMLParser), '
Apparently the XMLParser completely loses the <user> object, no longer being parsed when some unimportant whitespace is added.
The changes are invisible to the user making the edits. Often, users set their editors to convert tabs to spaces for coding style guidelines, and this can cause this bug to appear as a silent side-effect of other changes.
Updated by Marcos M about 2 months ago
- Category changed from Backup / Restore to Configuration Backend
- Status changed from New to Rejected
There are long-term plans that can help with the root issue here, but manual modification of the configuration file is not supported.