Merge pull request pencilblue#1207 from pencilblue/issue/1206

pencilblue#1206 fixed issue where unauthorized access of a route that did not h…
masete · Jan 23, 2017 · 23ff123 · 23ff123
2 parents 8a2eca3 + 4063666
commit 23ff123
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 4 deletions.
diff --git a/include/error/formatters/error_formatters.js b/include/error/formatters/error_formatters.js
@@ -17,6 +17,7 @@
 'use strict';
 
 //dependencies
+var _ = require('lodash');
 var path        = require('path');
 var HttpStatusCodes = require('http-status-codes');
 var XmlErrorFormatter = require('./xml_error_formatter');
@@ -145,9 +146,10 @@ module.exports = function(pb) {
                 failedControllerPaths[paths[i]] = true;
             }
         }
+
         params.request.controllerInstance = new ErrorController();
         params.request.controllerInstance.error = params.error;
-        params.request.themeRoute = params.request.themeRoute || {};
+        params.request.themeRoute = !!params.request.themeRoute ? _.clone(params.request.themeRoute) : {};
         params.request.routeTheme = params.request.routeTheme || {};
         params.request.siteObj = params.request.siteObj || pb.SiteService.getGlobalSiteContext();
         params.request.themeRoute.handler = 'render';

diff --git a/include/http/request_handler.js b/include/http/request_handler.js
@@ -485,8 +485,12 @@ module.exports = function RequestHandlerModule(pb) {
             routeDescriptor.themes[site][theme] = {};
             routeDescriptor.themes[site].size++;
         }
-        routeDescriptor.themes[site][theme][descriptor.method]            = descriptor;
-        routeDescriptor.themes[site][theme][descriptor.method].controller = Controller;
+
+        //set the controller then lock it down to prevent tampering
+        descriptor.controller = Controller;
+        routeDescriptor.themes[site][theme][descriptor.method] = Object.freeze(descriptor);
+
+
 
        //only add the descriptor it is new.  We do it here because we need to
        //know that the controller is good.

diff --git a/test/include/error/formatters/error_formatters_tests.js b/test/include/error/formatters/error_formatters_tests.js
@@ -78,19 +78,24 @@ describe('ErrorFormatters', function() {
             var error = new Error('hello world');
             error.code = 510;
 
+            var themeRoute = Object.freeze({
+                handler: 'testHandler'
+            });
             var params = {
                 error: error,
                 activeTheme: 'pencilblue',
                 request: {
                     router: {
                         continueAfter: function() {}
-                    }
+                    },
+                    themeRoute: themeRoute
                 }
             };
             sinon.spy(params.request.router, 'continueAfter');
             ErrorFormatters.html(params, function(err, result){});
             (typeof params.request.controllerInstance).should.eql('object');
             params.request.router.continueAfter.calledOnce.should.eql(true);
+            params.request.themeRoute.should.not.eql(themeRoute);
         });
     });