From 22c173192bb0dc189d8db84bbe8f0555b071f731 Mon Sep 17 00:00:00 2001
From: Tomas Jelinek <tojeline@redhat.com>
Date: Thu, 3 Sep 2015 17:14:05 +0200
Subject: [PATCH] fixed command injection vulnerability

---
 pcsd/fenceagent.rb | 18 ++++++++++++------
 pcsd/remote.rb     |  1 +
 pcsd/resource.rb   | 11 ++++++-----
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/pcsd/fenceagent.rb b/pcsd/fenceagent.rb
index 8b37147..c348597 100644
--- a/pcsd/fenceagent.rb
+++ b/pcsd/fenceagent.rb
@@ -18,12 +18,6 @@ def getFenceAgents(fence_agent = nil)
 end
 
 def getFenceAgentMetadata(fenceagentname)
-  # There are bugs in stonith_admin & the new fence_agents interaction
-  # eventually we'll want to switch back to this, but for now we directly
-  # call the agent to get metadata
-  #metadata = `stonith_admin --metadata -a #{fenceagentname}`
-  metadata = `/usr/sbin/#{fenceagentname} -o metadata`
-  doc = REXML::Document.new(metadata)
   options_required = {}
   options_optional = {}
   options_advanced = {
@@ -39,6 +33,18 @@ def getFenceAgentMetadata(fenceagentname)
     options_advanced["pcmk_" + a + "_timeout"] = ""
     options_advanced["pcmk_" + a + "_retries"] = ""
   end
+  # There are bugs in stonith_admin & the new fence_agents interaction
+  # eventually we'll want to switch back to this, but for now we directly
+  # call the agent to get metadata
+  #metadata = `stonith_admin --metadata -a #{fenceagentname}`
+  if not fenceagentname.start_with?('fence_') or fenceagentname.include?('/')
+    return [options_required, options_optional, options_advanced]
+  end
+  stdout, stderr, retval = run_cmd(
+    "/usr/sbin/#{fenceagentname}", '-o', 'metadata'
+  )
+  doc = REXML::Document.new(stdout.join)
+
   doc.elements.each('resource-agent/parameters/parameter') { |param|
     temp_array = []
     if param.elements["shortdesc"]
diff --git a/pcsd/remote.rb b/pcsd/remote.rb
index fe4a5d9..b3eca7e 100644
--- a/pcsd/remote.rb
+++ b/pcsd/remote.rb
@@ -840,6 +840,7 @@ def resource_metadata (params)
   return 200 if not params[:resourcename] or params[:resourcename] == ""
   resource_name = params[:resourcename][params[:resourcename].rindex(':')+1..-1]
   class_provider = params[:resourcename][0,params[:resourcename].rindex(':')]
+  return [400, 'Invalid resource agent name'] if resource_name.include?('/')
 
   @resource = ResourceAgent.new(params[:resourcename])
   if class_provider == "ocf:heartbeat"
diff --git a/pcsd/resource.rb b/pcsd/resource.rb
index 387e791..4e159f8 100644
--- a/pcsd/resource.rb
+++ b/pcsd/resource.rb
@@ -103,11 +103,12 @@ def getResourceOptions(resource_id,stonith=false)
 
   ret = {}
   if stonith
-    resource_options = `#{PCS} stonith show #{resource_id}`
+    command = [PCS, 'stonith', 'show', resource_id]
   else
-    resource_options = `#{PCS} resource show #{resource_id}`
+    command = [PCS, 'resource', 'show', resource_id]
   end
-  resource_options.each_line { |line|
+  stdout, stderr, retval = run_cmd(*command)
+  stdout.each { |line|
     keyval = line.strip.split(/: /,2)
     if keyval[0] == "Attributes" then
       options = keyval[1].split(/ /)
@@ -281,8 +282,8 @@ end
 
 def getResourceMetadata(resourcepath)
   ENV['OCF_ROOT'] = OCF_ROOT
-  metadata = `#{resourcepath} meta-data`
-  doc = REXML::Document.new(metadata)
+  stdout, stderr, retval = run_cmd(resourcepath, 'meta-data')
+  doc = REXML::Document.new(stdout.join)
   options_required = {}
   options_optional = {}
   long_desc = ""
-- 
1.9.1