CVE-2022-0587: Improper Authorization (CWE-285)
value: Improper Authorization in Packagist librenms/librenms prior to 22.2.0.
CWE description: The product does not perform or incorrectly performs an authorization check when an actor attempts to access a resource or perform an action.
CVSS v3 base score: 6.5
Published: 2022-02-15T08:15Z
Repository: https://github.com/librenms/librenms
Language: PHP
Files changed: includes/html/pages/alert-transports.inc.php, routes/web.php
Commit date: 2022-02-14 08:40:30 +0100
Commit message: Moved some pages to be within admin route (#13782)

* Moved plugin admin pages to be within admin route
* Wrap html transports page in admin check
* Moved Port group controller to be admin protected
* fixed tests
Diff: diff --git a/includes/html/pages/alert-transports.inc.php b/includes/html/pages/alert-transports.inc.php
index 9f9f431610..84390ccdfc 100644
--- a/includes/html/pages/alert-transports.inc.php
+++ b/includes/html/pages/alert-transports.inc.php
@@ -1,29 +1,33 @@
 <?php
 
-// handle OAuth requests
-$request = request();  // grab the Request object
+if (Auth::user()->hasGlobalAdmin()) {
+    // handle OAuth requests
+    $request = request();  // grab the Request object
 
-if ($request->has('oauthtransport')) {
-    // make sure transport is safe
-    $validator = Validator::make($request->all(), ['oauthtransport' => 'required|alpha']);
+    if ($request->has('oauthtransport')) {
+        // make sure transport is safe
+        $validator = Validator::make($request->all(), ['oauthtransport' => 'required|alpha']);
 
-    if ($validator->passes()) {
-        $transport_name = $request->get('oauthtransport');
-        $class = \LibreNMS\Alert\Transport::getClass($transport_name);
-        if (class_exists($class)) {
-            $transport = app($class);
-            if ($transport->handleOauth($request)) {
-                flash()->addSuccess("$transport_name added successfully.");
-            } else {
-                flash()->addError("$transport_name was not added. Check the log for details.");
+        if ($validator->passes()) {
+            $transport_name = $request->get('oauthtransport');
+            $class = \LibreNMS\Alert\Transport::getClass($transport_name);
+            if (class_exists($class)) {
+                $transport = app($class);
+                if ($transport->handleOauth($request)) {
+                    flash()->addSuccess("$transport_name added successfully.");
+                } else {
+                    flash()->addError("$transport_name was not added. Check the log for details.");
+                }
             }
         }
+
+        // remove get variables otherwise things will get double added
+        echo '<script>window.history.replaceState(null, null, window.location.pathname);</script>';
     }
+    unset($request);
 
-    // remove get variables otherwise things will get double added
-    echo '<script>window.history.replaceState(null, null, window.location.pathname);</script>';
+    // print alert transports
+    require_once 'includes/html/print-alert-transports.php';
+} else {
+    include 'includes/html/error-no-perm.inc.php';
 }
-unset($request);
-
-// print alert transports
-require_once 'includes/html/print-alert-transports.php';
diff --git a/routes/web.php b/routes/web.php
index 80cbb4ca99..3568ed1905 100644
--- a/routes/web.php
+++ b/routes/web.php
@@ -20,11 +20,10 @@ Auth::routes(['register' => false, 'reset' => false, 'verify' => false]);
 Route::group(['middleware' => ['auth'], 'guard' => 'auth'], function () {
 
     // pages
     Route::post('alert/{alert}/ack', [\App\Http\Controllers\AlertController::class, 'ack'])->name('alert.ack');
     Route::resource('device-groups', 'DeviceGroupController');
-    Route::resource('port-groups', 'PortGroupController');
     Route::resource('port', 'PortController', ['only' => 'update']);
     Route::group(['prefix' => 'poller'], function () {
         Route::get('', 'PollerController@pollerTab')->name('poller.index');
         Route::get('log', 'PollerController@logTab')->name('poller.log');
         Route::get('groups', 'PollerController@groupsTab')->name('poller.groups');
@@ -73,15 +72,18 @@ Route::group(['middleware' => ['auth'], 'guard' => 'auth'], function () {
         Route::get('settings/{tab?}/{section?}', 'SettingsController@index')->name('settings');
         Route::put('settings/{name}', 'SettingsController@update')->name('settings.update');
         Route::delete('settings/{name}', 'SettingsController@destroy')->name('settings.destroy');
 
         Route::post('alert/transports/{transport}/test', [\App\Http\Controllers\AlertTransportController::class, 'test'])->name('alert.transports.test');
+
+        Route::get('plugin/settings', 'PluginAdminController')->name('plugin.admin');
+        Route::get('plugin/settings/{plugin:plugin_name}', 'PluginSettingsController')->name('plugin.settings');
+        Route::post('plugin/settings/{plugin:plugin_name}', 'PluginSettingsController@update')->name('plugin.update');
+
+        Route::resource('port-groups', 'PortGroupController');
     });
 
-    Route::get('plugin/settings', 'PluginAdminController')->name('plugin.admin');
-    Route::get('plugin/settings/{plugin:plugin_name}', 'PluginSettingsController')->name('plugin.settings');
-    Route::post('plugin/settings/{plugin:plugin_name}', 'PluginSettingsController@update')->name('plugin.update');
     Route::get('plugin', 'PluginLegacyController@redirect');
     Route::redirect('plugin/view=admin', '/plugin/admin');
     Route::get('plugin/p={pluginName}', 'PluginLegacyController@redirect');
     Route::any('plugin/v1/{plugin:plugin_name}/{other?}', 'PluginLegacyController')->where('other', '(.*)')->name('plugin.legacy');
     Route::get('plugin/{plugin:plugin_name}', 'PluginPageController')->name('plugin.page');

Vulnerable code: // handle OAuth requests
$request = request();  // grab the Request object
if ($request->has('oauthtransport')) {
    // make sure transport is safe
    $validator = Validator::make($request->all(), ['oauthtransport' => 'required|alpha']);
    if ($validator->passes()) {
        $transport_name = $request->get('oauthtransport');
        $class = \LibreNMS\Alert\Transport::getClass($transport_name);
        if (class_exists($class)) {
            $transport = app($class);
            if ($transport->handleOauth($request)) {
                flash()->addSuccess("$transport_name added successfully.");
            } else {
                flash()->addError("$transport_name was not added. Check the log for details.");
    // remove get variables otherwise things will get double added
    echo '<script>window.history.replaceState(null, null, window.location.pathname);</script>';
unset($request);

// print alert transports
require_once 'includes/html/print-alert-transports.php';
    Route::resource('port-groups', 'PortGroupController');
    Route::get('plugin/settings', 'PluginAdminController')->name('plugin.admin');
    Route::get('plugin/settings/{plugin:plugin_name}', 'PluginSettingsController')->name('plugin.settings');
    Route::post('plugin/settings/{plugin:plugin_name}', 'PluginSettingsController@update')->name('plugin.update');
Fixed code: if (Auth::user()->hasGlobalAdmin()) {
    // handle OAuth requests
    $request = request();  // grab the Request object
    if ($request->has('oauthtransport')) {
        // make sure transport is safe
        $validator = Validator::make($request->all(), ['oauthtransport' => 'required|alpha']);
        if ($validator->passes()) {
            $transport_name = $request->get('oauthtransport');
            $class = \LibreNMS\Alert\Transport::getClass($transport_name);
            if (class_exists($class)) {
                $transport = app($class);
                if ($transport->handleOauth($request)) {
                    flash()->addSuccess("$transport_name added successfully.");
                } else {
                    flash()->addError("$transport_name was not added. Check the log for details.");
                }

        // remove get variables otherwise things will get double added
        echo '<script>window.history.replaceState(null, null, window.location.pathname);</script>';
    unset($request);
    // print alert transports
    require_once 'includes/html/print-alert-transports.php';
} else {
    include 'includes/html/error-no-perm.inc.php';

        Route::get('plugin/settings', 'PluginAdminController')->name('plugin.admin');
        Route::get('plugin/settings/{plugin:plugin_name}', 'PluginSettingsController')->name('plugin.settings');
        Route::post('plugin/settings/{plugin:plugin_name}', 'PluginSettingsController@update')->name('plugin.update');

        Route::resource('port-groups', 'PortGroupController');