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.
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.(There is a lot if "isnumeric()" stuff used around $id. I inherited this codebase.)
I assume this is only true in the context of authenticated sessions within the web panel, right? If that's the case, is there really a lot of damage an attacker could do with RCE in an authenticated session that they couldn't already do with admin access in the portal? I could be wrong, but it seems the security implications would be roughly the same just by virtue of providing a web admin tool with the ability to perform almost any superuser-level action.