diff --git a/openc3-cosmos-cmd-tlm-api/app/controllers/application_controller.rb b/openc3-cosmos-cmd-tlm-api/app/controllers/application_controller.rb index f3f81c9a1c..d09f6dbf51 100644 --- a/openc3-cosmos-cmd-tlm-api/app/controllers/application_controller.rb +++ b/openc3-cosmos-cmd-tlm-api/app/controllers/application_controller.rb @@ -57,4 +57,34 @@ def authorization(permission, target_name: nil) end true end + + def sanitize_params(param_list, require_params: true, allow_forward_slash: false) + if require_params + result = params.require(param_list) + else + result = [] + param_list.each do |param| + result << params[param] + end + end + result.each_with_index do |arg, index| + if arg + # Prevent the code scanner detects: + # "Uncontrolled data used in path expression" + # This method is taken directly from the Rails source: + # https://api.rubyonrails.org/v5.2/classes/ActiveStorage/Filename.html#method-i-sanitized + if allow_forward_slash + # Sometimes we have forward slashes so optionally allow those + value = arg.encode(Encoding::UTF_8, invalid: :replace, undef: :replace, replace: "�").strip.tr("\u{202E}%$|:;\t\r\n\\", "-") + else + value = arg.encode(Encoding::UTF_8, invalid: :replace, undef: :replace, replace: "�").strip.tr("\u{202E}%$|:;/\t\r\n\\", "-") + end + if value != arg + render(json: { status: 'error', message: "Invalid parameter #{param_list[index]}" }, status: 400) + return false + end + end + end + return result + end end diff --git a/openc3-cosmos-cmd-tlm-api/app/controllers/plugins_controller.rb b/openc3-cosmos-cmd-tlm-api/app/controllers/plugins_controller.rb index f740199161..81f615b537 100644 --- a/openc3-cosmos-cmd-tlm-api/app/controllers/plugins_controller.rb +++ b/openc3-cosmos-cmd-tlm-api/app/controllers/plugins_controller.rb @@ -35,14 +35,17 @@ def create(update = false) return unless authorization('admin') file = params[:plugin] if file + scope = sanitize_params([:scope]) + return unless scope + scope = scope[0] temp_dir = Dir.mktmpdir begin gem_file_path = temp_dir + '/' + file.original_filename FileUtils.cp(file.tempfile.path, gem_file_path) if @existing_model - result = OpenC3::PluginModel.install_phase1(gem_file_path, existing_variables: @existing_model['variables'], existing_plugin_txt_lines: @existing_model['plugin_txt_lines'], scope: params[:scope]) + result = OpenC3::PluginModel.install_phase1(gem_file_path, existing_variables: @existing_model['variables'], existing_plugin_txt_lines: @existing_model['plugin_txt_lines'], scope: scope) else - result = OpenC3::PluginModel.install_phase1(gem_file_path, scope: params[:scope]) + result = OpenC3::PluginModel.install_phase1(gem_file_path, scope: scope) end render :json => result rescue Exception => error @@ -67,6 +70,9 @@ def update def install return unless authorization('admin') begin + scope = sanitize_params([:scope]) + return unless scope + scope = scope[0] temp_dir = Dir.mktmpdir plugin_hash_filename = Dir::Tmpname.create(['plugin-instance-', '.json']) {} plugin_hash_file_path = File.join(temp_dir, File.basename(plugin_hash_filename)) @@ -74,10 +80,12 @@ def install file.write(params[:plugin_hash]) end - gem_name = params[:id].split("__")[0] + gem_name = sanitize_params([:id]) + return unless gem_name + gem_name = gem_name[0].split("__")[0] result = OpenC3::ProcessManager.instance.spawn( - ["ruby", "/openc3/bin/openc3cli", "load", gem_name, params[:scope], plugin_hash_file_path, "force"], # force install - "plugin_install", params[:id], Time.now + 1.hour, temp_dir: temp_dir, scope: params[:scope] + ["ruby", "/openc3/bin/openc3cli", "load", gem_name, scope, plugin_hash_file_path, "force"], # force install + "plugin_install", params[:id], Time.now + 1.hour, temp_dir: temp_dir, scope: scope ) render :json => result.name rescue Exception => error @@ -88,7 +96,9 @@ def install def destroy return unless authorization('admin') begin - result = OpenC3::ProcessManager.instance.spawn(["ruby", "/openc3/bin/openc3cli", "unload", params[:id], params[:scope]], "plugin_uninstall", params[:id], Time.now + 1.hour, scope: params[:scope]) + id, scope = sanitize_params([:id, scope]) + return unless id and scope + result = OpenC3::ProcessManager.instance.spawn(["ruby", "/openc3/bin/openc3cli", "unload", id, scope], "plugin_uninstall", id, Time.now + 1.hour, scope: scope) render :json => result.name rescue Exception => error render(:json => { :status => 'error', :message => error.message }, :status => 500) and return diff --git a/openc3-cosmos-cmd-tlm-api/app/controllers/screens_controller.rb b/openc3-cosmos-cmd-tlm-api/app/controllers/screens_controller.rb index 0b40d1e3e0..354d6157bd 100644 --- a/openc3-cosmos-cmd-tlm-api/app/controllers/screens_controller.rb +++ b/openc3-cosmos-cmd-tlm-api/app/controllers/screens_controller.rb @@ -19,12 +19,17 @@ class ScreensController < ApplicationController def index return unless authorization('system') - render :json => Screen.all(*params.require([:scope])) + scope = sanitize_params([:scope]) + return unless scope + scope = scope[0] + render :json => Screen.all(scope) end def show return unless authorization('system') - screen = Screen.find(*params.require([:scope, :target, :screen])) + result = sanitize_params([:scope, :target, :screen]) + return unless result + screen = Screen.find(*result) if screen render :json => screen else @@ -34,7 +39,11 @@ def show def create return unless authorization('system_set') - screen = Screen.create(*params.require([:scope, :target, :screen, :text])) + result = sanitize_params([:scope, :target, :screen]) + return unless result + text = params.require([:text])[0] + result << text + screen = Screen.create(*result) OpenC3::Logger.info("Screen saved: #{params[:target]} #{params[:screen]}", scope: params[:scope], user: username()) render :json => screen rescue => e @@ -43,7 +52,9 @@ def create def destroy return unless authorization('system_set') - screen = Screen.destroy(*params.require([:scope, :target, :screen])) + result = sanitize_params([:scope, :target, :screen]) + return unless result + screen = Screen.destroy(*result) OpenC3::Logger.info("Screen deleted: #{params[:target]} #{params[:screen]}", scope: params[:scope], user: username()) head :ok rescue => e diff --git a/openc3-cosmos-cmd-tlm-api/app/controllers/storage_controller.rb b/openc3-cosmos-cmd-tlm-api/app/controllers/storage_controller.rb index 3fb3b59953..ad321cccc0 100644 --- a/openc3-cosmos-cmd-tlm-api/app/controllers/storage_controller.rb +++ b/openc3-cosmos-cmd-tlm-api/app/controllers/storage_controller.rb @@ -25,6 +25,7 @@ class StorageController < ApplicationController def buckets + return unless authorization('system') # ENV.map returns a big array of mostly nils which is why we compact # The non-nil are MatchData objects due to the regex match matches = ENV.map { |key, _value| key.match(/^OPENC3_(.+)_BUCKET$/) }.compact @@ -35,6 +36,7 @@ def buckets end def volumes + return unless authorization('system') # ENV.map returns a big array of mostly nils which is why we compact # The non-nil are MatchData objects due to the regex match matches = ENV.map { |key, _value| key.match(/^OPENC3_(.+)_VOLUME$/) }.compact diff --git a/openc3-cosmos-cmd-tlm-api/app/controllers/tables_controller.rb b/openc3-cosmos-cmd-tlm-api/app/controllers/tables_controller.rb index 3369060933..faf0518a3e 100644 --- a/openc3-cosmos-cmd-tlm-api/app/controllers/tables_controller.rb +++ b/openc3-cosmos-cmd-tlm-api/app/controllers/tables_controller.rb @@ -23,17 +23,20 @@ require 'base64' class TablesController < ApplicationController - before_action :sanitize_scope - def index return unless authorization('system') - render json: Table.all(params[:scope]) + scope = sanitize_params([:scope]) + return unless scope + scope = scope[0] + render json: Table.all(scope) end def binary return unless authorization('system') + scope, binary, definition, table = sanitize_params([:scope, :binary, :definition, :table], require_params: false, allow_forward_slash: true) + return unless scope begin - file = Table.binary(params[:scope], params[:binary], params[:definition], params[:table]) + file = Table.binary(scope, binary, definition, table) results = { 'filename' => file.filename, 'contents' => Base64.encode64(file.contents) } render json: results rescue Table::NotFound => e @@ -44,8 +47,10 @@ def binary def definition return unless authorization('system') + scope, definition, table = sanitize_params([:scope, :definition, :table], require_params: false, allow_forward_slash: true) + return unless scope begin - file = Table.definition(params[:scope], params[:definition], params[:table]) + file = Table.definition(scope, definition, table) render json: { 'filename' => file.filename, 'contents' => file.contents } rescue Table::NotFound => e render(json: { status: 'error', message: e.message }, status: 404) and @@ -55,8 +60,10 @@ def definition def report return unless authorization('system') + scope, binary, definition, table = sanitize_params([:scope, :binary, :definition, :table], require_params: false, allow_forward_slash: true) + return unless scope begin - file = Table.report(params[:scope], params[:binary], params[:definition], params[:table]) + file = Table.report(scope, binary, definition, table) render json: { 'filename' => file.filename, 'contents' => file.contents } rescue Table::NotFound => e render(json: { status: 'error', message: e.message }, status: 404) and @@ -66,17 +73,19 @@ def report def body return unless authorization('system') + scope, name = sanitize_params([:scope, :name], require_params: true, allow_forward_slash: true) + return unless scope # body doesn't raise if not found ... it returns nil - file = Table.body(params[:scope], params[:name]) + file = Table.body(scope, name) if file results = {} - if File.extname(params[:name]) == '.txt' + if File.extname(name) == '.txt' results = { 'contents' => file } else - locked = Table.locked?(params[:scope], params[:name]) + locked = Table.locked?(scope, name) unless locked - Table.lock(params[:scope], params[:name], username()) + Table.lock(scope, name, username()) end results = { 'contents' => Base64.encode64(file), 'locked' => locked } end @@ -91,8 +100,10 @@ def body def load return unless authorization('system') + scope, binary, definition = sanitize_params([:scope, :binary, :definition], require_params: false, allow_forward_slash: true) + return unless scope begin - render json: Table.load(params[:scope], params[:binary], params[:definition]) + render json: Table.load(scope, binary, definition) rescue Table::NotFound => e render(json: { status: 'error', message: e.message }, status: 404) and return @@ -101,8 +112,10 @@ def load def save return unless authorization('system') + scope, binary, definition = sanitize_params([:scope, :binary, :definition], require_params: false, allow_forward_slash: true) + return unless scope begin - Table.save(params[:scope], params[:binary], params[:definition], params[:tables]) + Table.save(scope, binary, definition, params[:tables]) head :ok rescue Table::NotFound => e render(json: { status: 'error', message: e.message }, status: 404) and @@ -112,8 +125,10 @@ def save def save_as return unless authorization('system') + scope, name, new_name = sanitize_params([:scope, :name, :new_name], require_params: true, allow_forward_slash: true) + return unless scope begin - Table.save_as(params[:scope], params[:name], params[:new_name]) + Table.save_as(scope, name, new_name) head :ok rescue Table::NotFound => e render(json: { status: 'error', message: e.message }, status: 404) and @@ -123,8 +138,10 @@ def save_as def generate return unless authorization('system') + scope, definition = sanitize_params([:scope, :definition], require_params: false, allow_forward_slash: true) + return unless scope begin - filename = Table.generate(params[:scope], params[:definition]) + filename = Table.generate(scope, definition) render json: { 'filename' => filename } rescue Table::NotFound => e render(json: { status: 'error', message: e.message }, status: 404) and @@ -134,40 +151,32 @@ def generate def lock return unless authorization('system') - Table.lock(params[:scope], params[:name], username()) + scope, name = sanitize_params([:scope, :name], require_params: true, allow_forward_slash: true) + return unless scope + Table.lock(scope, name, username()) render status: 200 end def unlock return unless authorization('system') - locked_by = Table.locked?(params[:scope], params[:name]) - Table.unlock(params[:scope], params[:name]) if username() == locked_by + scope, name = sanitize_params([:scope, :name], require_params: true, allow_forward_slash: true) + return unless scope + locked_by = Table.locked?(scope, name) + Table.unlock(scope, name) if username() == locked_by render status: 200 end def destroy return unless authorization('system') + scope, name = sanitize_params([:scope, :name], require_params: true, allow_forward_slash: true) + return unless scope # destroy returns no indication of success or failure so just assume it worked - Table.destroy(params[:scope], params[:name]) + Table.destroy(scope, name) OpenC3::Logger.info( - "Table destroyed: #{params[:name]}", - scope: params[:scope], + "Table destroyed: #{name}", + scope: scope, user: username() ) head :ok end - - private - - def sanitize_scope - # scope is passed as a parameter and we use it to create paths in local_mode, - # thus we have to sanitize it or the code scanner detects: - # "Uncontrolled data used in path expression" - # This method is taken directly from the Rails source: - # https://api.rubyonrails.org/v5.2/classes/ActiveStorage/Filename.html#method-i-sanitized - scope = params[:scope].encode(Encoding::UTF_8, invalid: :replace, undef: :replace, replace: "�").strip.tr("\u{202E}%$|:;/\t\r\n\\", "-") - if scope != params[:scope] - render(json: { status: 'error', message: "Invalid scope: #{params[:scope]}" }, status: 400) - end - end end diff --git a/openc3-cosmos-cmd-tlm-api/app/controllers/targets_controller.rb b/openc3-cosmos-cmd-tlm-api/app/controllers/targets_controller.rb index 479bb0e102..0fa470a9d3 100644 --- a/openc3-cosmos-cmd-tlm-api/app/controllers/targets_controller.rb +++ b/openc3-cosmos-cmd-tlm-api/app/controllers/targets_controller.rb @@ -23,8 +23,6 @@ require 'openc3/models/target_model' class TargetsController < ModelController - before_action :sanitize_scope - def initialize @model_class = OpenC3::TargetModel end @@ -32,34 +30,43 @@ def initialize # All targets with indication of modified targets def all_modified return unless authorization('system') - render :json => @model_class.all_modified(scope: params[:scope]) + scope = sanitize_params([:scope], require_params: true) + return unless scope + scope = scope[0] + render :json => @model_class.all_modified(scope: scope) end def modified_files return unless authorization('system') + scope, id = sanitize_params([:scope, :id], require_params: true) + return unless scope begin - render :json => @model_class.modified_files(params[:id], scope: params[:scope]) + render :json => @model_class.modified_files(id, scope: scope) rescue Exception => e - OpenC3::Logger.info("Target '#{params[:id]} modified_files failed: #{e.message}", user: username()) + OpenC3::Logger.info("Target '#{id} modified_files failed: #{e.message}", user: username()) head :internal_server_error end end def delete_modified return unless authorization('system') + scope, id = sanitize_params([:scope, :id], require_params: true) + return unless scope begin - @model_class.delete_modified(params[:id], scope: params[:scope]) + @model_class.delete_modified(id, scope: scope) head :ok rescue Exception => e - OpenC3::Logger.info("Target '#{params[:id]} delete_modified failed: #{e.message}", user: username()) + OpenC3::Logger.info("Target '#{id} delete_modified failed: #{e.message}", user: username()) head :internal_server_error end end def download return unless authorization('system') + scope, id = sanitize_params([:scope, :id], require_params: true) + return unless scope begin - file = @model_class.download(params[:id], scope: params[:scope]) + file = @model_class.download(id, scope: scope) if file results = { 'filename' => file.filename, 'contents' => Base64.encode64(file.contents) } render json: results @@ -67,22 +74,8 @@ def download head :not_found end rescue Exception => e - OpenC3::Logger.info("Target '#{params[:id]} download failed: #{e.message}", user: username()) + OpenC3::Logger.info("Target '#{id} download failed: #{e.message}", user: username()) render(json: { status: 'error', message: e.message }, status: 500) and return end end - - private - - def sanitize_scope - # scope is passed as a parameter and we use it to create paths in local_mode, - # thus we have to sanitize it or the code scanner detects: - # "Uncontrolled data used in path expression" - # This method is taken directly from the Rails source: - # https://api.rubyonrails.org/v5.2/classes/ActiveStorage/Filename.html#method-i-sanitized - scope = params[:scope].encode(Encoding::UTF_8, invalid: :replace, undef: :replace, replace: "�").strip.tr("\u{202E}%$|:;/\t\r\n\\", "-") - if scope != params[:scope] - render(json: { status: 'error', message: "Invalid scope: #{params[:scope]}" }, status: 400) - end - end end diff --git a/openc3-cosmos-script-runner-api/app/controllers/application_controller.rb b/openc3-cosmos-script-runner-api/app/controllers/application_controller.rb index 6618630bb1..95dca89205 100644 --- a/openc3-cosmos-script-runner-api/app/controllers/application_controller.rb +++ b/openc3-cosmos-script-runner-api/app/controllers/application_controller.rb @@ -67,4 +67,34 @@ def authorization(permission, target_name: nil) end true end + + def sanitize_params(param_list, require_params: true, allow_forward_slash: false) + if require_params + result = params.require(param_list) + else + result = [] + param_list.each do |param| + result << params[param] + end + end + result.each_with_index do |arg, index| + if arg + # Prevent the code scanner detects: + # "Uncontrolled data used in path expression" + # This method is taken directly from the Rails source: + # https://api.rubyonrails.org/v5.2/classes/ActiveStorage/Filename.html#method-i-sanitized + if allow_forward_slash + # Sometimes we have forward slashes so optionally allow those + value = arg.encode(Encoding::UTF_8, invalid: :replace, undef: :replace, replace: "�").strip.tr("\u{202E}%$|:;\t\r\n\\", "-") + else + value = arg.encode(Encoding::UTF_8, invalid: :replace, undef: :replace, replace: "�").strip.tr("\u{202E}%$|:;/\t\r\n\\", "-") + end + if value != arg + render(json: { status: 'error', message: "Invalid parameter #{param_list[index]}" }, status: 400) + return false + end + end + end + return result + end end diff --git a/openc3-cosmos-script-runner-api/app/controllers/scripts_controller.rb b/openc3-cosmos-script-runner-api/app/controllers/scripts_controller.rb index 41663bd65f..e4393590af 100644 --- a/openc3-cosmos-script-runner-api/app/controllers/scripts_controller.rb +++ b/openc3-cosmos-script-runner-api/app/controllers/scripts_controller.rb @@ -40,31 +40,39 @@ def ping def index return unless authorization('script_view') - render :json => Script.all(params[:scope]) + scope = sanitize_params([:scope]) + return unless scope + scope = scope[0] + render :json => Script.all(scope) end def delete_temp return unless authorization('script_edit') - render :json => Script.delete_temp(params[:scope]) + scope = sanitize_params([:scope]) + return unless scope + scope = scope[0] + render :json => Script.delete_temp(scope) end def body return unless authorization('script_view') + scope, name = sanitize_params([:scope, :name]) + return unless scope - file = Script.body(params[:scope], params[:name]) + file = Script.body(scope, name) if file - locked = Script.locked?(params[:scope], params[:name]) + locked = Script.locked?(scope, name) unless locked - Script.lock(params[:scope], params[:name], username()) + Script.lock(scope, name, username()) end - breakpoints = Script.get_breakpoints(params[:scope], params[:name]) + breakpoints = Script.get_breakpoints(scope, name) results = { contents: file, breakpoints: breakpoints, locked: locked } - if ((File.extname(params[:name]) == '.py') and (file =~ PYTHON_SUITE_REGEX)) or ((File.extname(params[:name]) != '.py') and (file =~ SUITE_REGEX)) - results_suites, results_error, success = Script.process_suite(params[:name], file, username: username(), scope: params[:scope]) + if ((File.extname(name) == '.py') and (file =~ PYTHON_SUITE_REGEX)) or ((File.extname(name) != '.py') and (file =~ SUITE_REGEX)) + results_suites, results_error, success = Script.process_suite(name, file, username: username(), scope: scope) results['suites'] = results_suites results['error'] = results_error results['success'] = success @@ -79,30 +87,37 @@ def body def create return unless authorization('script_edit') - Script.create(params.permit(:scope, :name, :text, breakpoints: [])) + scope, name = sanitize_params([:scope, :name], :allow_forward_slash => true) + return unless scope + args = params.permit(:text, breakpoints: []) + args[:scope] = scope + args[:name] = name + Script.create(args) results = {} - if ((File.extname(params[:name]) == '.py') and (params[:text] =~ PYTHON_SUITE_REGEX)) or ((File.extname(params[:name]) != '.py') and (params[:text] =~ SUITE_REGEX)) - results_suites, results_error, success = Script.process_suite(params[:name], params[:text], username: username(), scope: params[:scope]) + if ((File.extname(name) == '.py') and (params[:text] =~ PYTHON_SUITE_REGEX)) or ((File.extname(name) != '.py') and (params[:text] =~ SUITE_REGEX)) + results_suites, results_error, success = Script.process_suite(name, params[:text], username: username(), scope: scope) results['suites'] = results_suites results['error'] = results_error results['success'] = success end - OpenC3::Logger.info("Script created: #{params[:name]}", scope: params[:scope], user: username()) if success + OpenC3::Logger.info("Script created: #{name}", scope: scope, user: username()) if success render :json => results rescue => e render(json: { status: 'error', message: e.message }, status: 500) end def run + scope, name = sanitize_params([:scope, :name], :allow_forward_slash => true) + return unless scope # Extract the target that this script lives under - target_name = params[:name].split('/')[0] + target_name = name.split('/')[0] return unless authorization('script_run', target_name: target_name) suite_runner = params[:suiteRunner] ? params[:suiteRunner].as_json(:allow_nan => true) : nil disconnect = params[:disconnect] == 'disconnect' environment = params[:environment] - running_script_id = Script.run(params[:scope], params[:name], suite_runner, disconnect, environment, user_full_name(), username()) + running_script_id = Script.run(scope, name, suite_runner, disconnect, environment, user_full_name(), username()) if running_script_id - OpenC3::Logger.info("Script started: #{params[:name]}", scope: params[:scope], user: username()) + OpenC3::Logger.info("Script started: #{name}", scope: scope, user: username()) render :plain => running_script_id.to_s else head :not_found @@ -111,21 +126,27 @@ def run def lock return unless authorization('script_edit') - Script.lock(params[:scope], params[:name], username()) + scope, name = sanitize_params([:scope, :name]) + return unless scope + Script.lock(scope, name, username()) render status: 200 end def unlock return unless authorization('script_edit') - locked_by = Script.locked?(params[:scope], params[:name]) - Script.unlock(params[:scope], params[:name]) if username() == locked_by + scope, name = sanitize_params([:scope, :name]) + return unless scope + locked_by = Script.locked?(scope, name) + Script.unlock(scope, name) if username() == locked_by render status: 200 end def destroy return unless authorization('script_edit') - Script.destroy(*params.require([:scope, :name])) - OpenC3::Logger.info("Script destroyed: #{params[:name]}", scope: params[:scope], user: username()) + scope, name = sanitize_params([:scope, :name]) + return unless scope + Script.destroy(scope, name) + OpenC3::Logger.info("Script destroyed: #{name}", scope: scope, user: username()) head :ok rescue => e render(json: { status: 'error', message: e.message }, status: 500) @@ -133,9 +154,12 @@ def destroy def syntax # Extract the target that this script lives under - target_name = params[:name].split('/')[0] + name = sanitize_params([:name], :allow_forward_slash => true) + return unless name + name = name[0] + target_name = name.split('/')[0] return unless authorization('script_run', target_name: target_name) - script = Script.syntax(params[:name], request.body.read) + script = Script.syntax(name, request.body.read) if script render :json => script else @@ -145,7 +169,10 @@ def syntax def instrumented return unless authorization('script_view') - script = Script.instrumented(params[:name], request.body.read) + name = sanitize_params([:name], :allow_forward_slash => true) + return unless name + name = name[0] + script = Script.instrumented(name, request.body.read) if script render :json => script else @@ -155,7 +182,10 @@ def instrumented def delete_all_breakpoints return unless authorization('script_edit') - OpenC3::Store.del("#{params[:scope]}__script-breakpoints") + scope = sanitize_params([:scope]) + return unless scope + scope = scope[0] + OpenC3::Store.del("#{scope}__script-breakpoints") head :ok end end