Skip to content

Commit

Permalink
Merge pull request #20130 from d-m-u/use_awesome_spawn_for_apache_stop
Browse files Browse the repository at this point in the history
fix brakeman warning about possible command injection
  • Loading branch information
carbonin authored May 7, 2020
2 parents 927b99f + 14dab93 commit 34bfb38
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 21 deletions.
20 changes: 0 additions & 20 deletions config/brakeman.ignore
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
2 changes: 1 addition & 1 deletion lib/miq_apache/control.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 34bfb38

Please sign in to comment.