From 14dab93d42baabfd1d4e04606a9da71a2a57f1aa Mon Sep 17 00:00:00 2001 From: d-m-u Date: Wed, 6 May 2020 12:12:55 -0400 Subject: [PATCH] fix brakeman warning about possible command injection --- config/brakeman.ignore | 20 -------------------- lib/miq_apache/control.rb | 2 +- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/config/brakeman.ignore b/config/brakeman.ignore index de2755302c0..96a5f093234 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -1,25 +1,5 @@ { "ignored_warnings": [ - { - "warning_type": "Command Injection", - "warning_code": 14, - "fingerprint": "14cf1baf5a62a0f109a2deb71920db195735f3054b031e50513d433f1566a670", - "check_name": "Execute", - "message": "Possible command injection", - "file": "lib/miq_apache/control.rb", - "line": 55, - "link": "http://brakemanscanner.org/docs/warning_types/command_injection/", - "code": "system(\"kill -WINCH #{`pgrep -P 1 httpd`.chomp.to_i}\")", - "render_path": null, - "location": { - "type": "method", - "class": "MiqApache::Control", - "method": "s(:self).stop" - }, - "user_input": "`pgrep -P 1 httpd`.chomp.to_i", - "confidence": "Medium", - "note": "The chomp.to_i ensures we get a number and we protect against 0 with a conditional. The only other possible avenue for attack is if the attacker could replace pgrep, but then they already have root access, so it's a moot point." - }, { "warning_type": "File Access", "warning_code": 16, diff --git a/lib/miq_apache/control.rb b/lib/miq_apache/control.rb index 2e29064a14e..df357efdec6 100644 --- a/lib/miq_apache/control.rb +++ b/lib/miq_apache/control.rb @@ -52,7 +52,7 @@ def self.start def self.stop if ENV["CONTAINER"] pid = `pgrep -P 1 httpd`.chomp.to_i - system("kill -WINCH #{pid}") if pid > 0 + Process.kill("WINCH", pid) if pid > 0 else run_apache_cmd('stop') end