Yes, RCE allows an attacker to install a permanent bot/rootkit/whatever. XSS and CSRS can allow the attacks to occur silently. Put them together and you can get hacked just by browsing the web, without ever realizing it.
If I remember correctly, the web panel allows installing packages from custom repository sources. I'm not saying that's inherently secure (it obviously makes other vulnerabilities much more critical), but how is it realistically different from arbitrary RCE within authenticated sessions?
I agree that CSRF and XSS vulnerabilities - when combined with superuser-level access of any kind - are basically "game over" for security, but from what I've seen, pfSense has been reasonably quick to patch any such vulnerabilities in the past. XSS, while still applicable, is less likely in a system where very little third-party content is rendered in the browser. When it comes to CSRF, I believe most security-conscious administrators keep things like pfSense web sessions limited, logging out before resuming any other browsing (or better, using it only from a separate browser profile). That doesn't solve those issues, but it can limit the attack surface.
Not trying to defend pfSense's security as a whole; I'm just pointing out that RCE is somewhat irrelevant when we're talking about a tool that already allows so much access anyway. One should essentially treat a pfSense admin session as if it were a web-based root shell (because it essentially is that).
A web admin tool that can perform superuser actions is - and probably always will be - less secure than other alternatives. Security-conscious people understand that, and will skip tools like pfSense in high-risk environments. But for many others, those risks are reasonable to accept for the added convenience.
There is actually work going into 2.4 that should further rectify the situation.
pfSenseHelpers.js (https://github.com/pfsense/pfsense/commit/f0136b178022cf3b04...) now contains code that intercepts clicks on anchor tags with the attribute "usepost" set. The target URL and the GET arguments are extracted from the event href attribute, and these are used to compose a new, temporary form with the previous arguments inserted as POST parameters.
Here is a simple example:
Before:
<?php
$id = $_GET['id'];
if ($_POST) {
if (!save_config($id)) {
$input_errors[] = "Something broke";
}
}
?>
<a type="button" href="system_something.php?id=<?=$id?>&user=<?=$pconfig['user']?> >
<i class="fa fa-save></i>
<?=gettext("Save")?>
</a>
After:
<?php
$id = $_POST['id']; // $_GET was changed to $_POST
if ($_POST['save']) { // The generic if ($_POST) is now if ($_POST['save'] to detect when the form is being saved
if (!save_config($id)) {
$input_errors[] = "Something broke";
}
}
?>
<!-- The "usepost" attribute is added to the anchor -->
<a type="button" href="system_something.php?id=<?=$id?>&user=<?=$pconfig['user']?> usepost>
<i class="fa fa-save></i>
<?=gettext("Save")?>
</a>
Most of the GUI is being converted to use these, and the result should be in pfSense software release 2.4.
Why isn't `$id` HTML-escaped before being echoed in an HTML attribute context? That's a potential XSS vector. Slightly more difficult to exploit when pulled from `$_POST`, but still a problem.
I don't mean to be combative, but when providing an example of how you're planning to mitigate vulnerabilities, it seems unwise to use code that contains an easily avoidable XSS vuln. I love pfSense and have used it for years, but this sort of thing doesn't inspire confidence.
Understood. It wasn't provided in the example, because it is not a user-entered item and is rarely displayed on the screen. It is typically an index. In other cases (perhaps) because people who originally wrote it knew no better.
(There is a lot if "isnumeric()" stuff used around $id. I inherited this codebase.)
