Landing #12980, ensure json is always returned from the json rpc api

lib/msf/core/rpc.rb

@@ -1,4 +1,5 @@
 # -*- coding: binary -*-
+
 module Msf::RPC
   require 'msf/core/rpc/v10/constants'
 
@@ -40,6 +41,5 @@ module Msf::RPC
     autoload :InvalidResponse, 'msf/core/rpc/json/error'
     autoload :JSONParseError, 'msf/core/rpc/json/error'
     autoload :ErrorResponse, 'msf/core/rpc/json/error'
-
   end
 end

lib/msf/core/rpc/json/error.rb

@@ -14,14 +14,14 @@ module Msf::RPC::JSON
 
   # JSON-RPC 2.0 Error Messages
   ERROR_MESSAGES = {
-      # Specification errors:
-      PARSE_ERROR => 'Invalid JSON was received by the server. An error occurred on the server while parsing the JSON text.',
-      INVALID_REQUEST => 'The JSON sent is not a valid Request object.',
-      METHOD_NOT_FOUND => 'The method %<name>s does not exist.',
-      INVALID_PARAMS => 'Invalid method parameter(s).',
-      INTERNAL_ERROR => 'Internal JSON-RPC error',
-      # Implementation-defined server-errors:
-      APPLICATION_SERVER_ERROR => 'Application server error: %<msg>s',
+    # Specification errors:
+    PARSE_ERROR => 'Invalid JSON was received by the server. An error occurred on the server while parsing the JSON text.',
+    INVALID_REQUEST => 'The JSON sent is not a valid Request object.',
+    METHOD_NOT_FOUND => 'The method %<name>s does not exist.',
+    INVALID_PARAMS => 'Invalid method parameter(s).',
+    INTERNAL_ERROR => 'Internal JSON-RPC error',
+    # Implementation-defined server-errors:
+    APPLICATION_SERVER_ERROR => 'Application server error: %<msg>s'
   }
 
   # Base class for all Msf::RPC::JSON exceptions.

lib/msf/core/web_services/json_rpc_app.rb

@@ -8,6 +8,7 @@ require 'msf/core/web_services/framework_extension'
 require 'msf/core/web_services/servlet_helper'
 require 'msf/core/web_services/servlet/auth_servlet'
 require 'msf/core/web_services/servlet/json_rpc_servlet'
+require 'msf/core/web_services/json_rpc_exception_handling'
 
 module Msf::WebServices
   class JsonRpcApp < Sinatra::Base
@@ -21,9 +22,15 @@ module Msf::WebServices
     register AuthServlet
     register JsonRpcServlet
 
+    # Custom error handling
+    register JsonRpcExceptionHandling::SinatraExtension
+
     configure do
       set :dispatchers, {}
 
+      # Disables Sinatra HTML Error Responses
+      set :show_exceptions, false
+
       set :sessions, {key: 'msf-ws.session', expire_after: 300}
       set :session_secret, ENV.fetch('MSF_WS_SESSION_SECRET', SecureRandom.hex(16))
       set :api_token, ENV.fetch('MSF_WS_JSON_RPC_API_TOKEN', nil)
@@ -85,5 +92,11 @@ module Msf::WebServices
       false
     end
 
+    def self.setup_default_middleware(builder)
+      super
+      # Insertion at pos 1 needed to immediately follow Sinatra::ExtendedBase
+      # proc block identical to one used in 'use' method lib/rack/builder:86
+      builder.instance_variable_get(:@use).insert(1, proc { |app| JsonRpcExceptionHandling::RackMiddleware.new(app) })
+    end
   end
 end

lib/msf/core/web_services/json_rpc_exception_handling.rb

@@ -0,0 +1,76 @@
+require 'msf/core/rpc'
+
+module Msf::WebServices::JsonRpcExceptionHandling
+  class RackMiddleware
+    def initialize(app)
+      @app = app
+    end
+
+    def call(env)
+      begin
+        @app.call(env)
+      rescue Exception => e
+        req = Rack::Request.new(env)
+        ErrorHandler.get_response(e, req)
+      end
+    end
+  end
+
+  module SinatraExtension
+    def self.registered(app)
+      app.error do |err|
+        ErrorHandler.get_response(err, self.request)
+      end
+    end
+  end
+
+  class ErrorHandler
+    class << self
+      def get_response(err, request)
+        parsed_request = parse_request(request)
+        data = get_data(err)
+
+        response = Msf::RPC::JSON::Dispatcher::create_error_response(
+          Msf::RPC::JSON::ApplicationServerError.new(
+            err,
+            data: data
+          ),
+          parsed_request
+        )
+
+        Rack::Response.new(
+          response.to_json,
+          500,
+          {'Content-type' => 'application/json'}
+        ).finish
+      end
+
+      private
+
+      def get_data(err)
+        return nil unless development?
+
+        {
+          "backtrace" => err.backtrace
+        }
+      end
+
+      def parse_request(req)
+        begin
+          body = req.body.tap(&:rewind).read
+          JSON.parse(body, symbolize_names: true)
+        rescue JSON::ParserError
+          nil
+        end
+      end
+
+      def development?
+        environment == :development
+      end
+
+      def environment
+        (ENV['RACK_ENV'] || :development).to_sym
+      end
+    end
+  end
+end

spec/api/json_rpc_spec.rb

@@ -1,5 +1,7 @@
 require 'spec_helper'
+require 'msf/core/rpc'
 require 'rack/test'
+require 'rack/protection'
 
 # These tests ensure the full end to end functionality of metasploit's JSON RPC
 # endpoint. There are multiple layers of possible failure in our API, and unit testing
@@ -60,6 +62,16 @@ RSpec.describe "Metasploit's json-rpc" do
     expect(rpc_response).to include({ 'result' => hash_including({ 'status' => 'errored' }) })
   end
 
+  def mock_rack_env(mock_rack_env_value)
+    allow(ENV).to receive(:[]).and_wrap_original do |original_env, key|
+      if key == 'RACK_ENV'
+        mock_rack_env_value
+      else
+        original_env[key]
+      end
+    end
+  end
+
   # Waits until the given expectations are all true. This function executes the given block,
   # and if a failure occurs it will be retried `retry_count` times before finally failing.
   # This is useful to expect against asynchronous/eventually consistent systems.
@@ -179,5 +191,104 @@ RSpec.describe "Metasploit's json-rpc" do
         expect(last_json_response).to include(expected_error_response)
       end
     end
+
+    context "when there is a sinatra level application error in the development environment" do
+      before(:each) do
+        allow_any_instance_of(Msf::RPC::JSON::Dispatcher).to receive(:process) do
+          raise Exception, "Sinatra level exception raised"
+        end
+        mock_rack_env("development")
+      end
+
+      it 'returns the error results' do
+        create_job
+
+        expect(last_response).to be_server_error
+        expected_error_response = {
+          "error" => {
+            "code" => -32000,
+            "data" => {
+              "backtrace" => include(a_kind_of(String))
+            },
+            "message" => "Application server error: Sinatra level exception raised"
+          },
+          "id" => 1
+        }
+        expect(last_json_response).to include(expected_error_response)
+      end
+    end
+
+    context "when rack middleware raises an error in the development environment" do
+      before(:each) do
+        allow_any_instance_of(::Rack::Protection::AuthenticityToken).to receive(:accepts?) do
+          raise Exception, "Middleware error raised"
+        end
+
+        mock_rack_env("development")
+      end
+
+      it 'returns the error results' do
+        create_job
+
+        expect(last_response).to be_server_error
+        expected_error_response = {
+          "error" => {
+            "code" => -32000,
+            "data" => {
+              "backtrace" => include(a_kind_of(String))
+            },
+            "message" => "Application server error: Middleware error raised"
+          },
+          "id" => 1
+        }
+        expect(last_json_response).to include(expected_error_response)
+      end
+    end
+
+    context "when rack middleware raises an error in the production environment" do
+      before(:each) do
+        allow_any_instance_of(::Rack::Protection::AuthenticityToken).to receive(:accepts?) do
+          raise Exception, "Middleware error raised"
+        end
+        mock_rack_env("production")
+      end
+
+      it 'returns the error results' do
+        create_job
+
+        expect(last_response).to be_server_error
+        expected_error_response = {
+          "error" => {
+            "code" => -32000,
+            "message" => "Application server error: Middleware error raised"
+          },
+          "id" => 1
+        }
+        expect(last_json_response).to include(expected_error_response)
+      end
+    end
+
+    context "when there is a sinatra level application error in the production environment" do
+      before(:each) do
+        allow_any_instance_of(Msf::RPC::JSON::Dispatcher).to receive(:process) do
+          raise Exception, "Sinatra level exception raised"
+        end
+        mock_rack_env("production")
+      end
+
+      it 'returns the error results' do
+        create_job
+
+        expect(last_response).to be_server_error
+        expected_error_response = {
+          "error" => {
+            "code" => -32000,
+            "message" => "Application server error: Sinatra level exception raised"
+          },
+          "id" => 1
+        }
+        expect(last_json_response).to include(expected_error_response)
+      end
+    end
   end
 end