Hammerspoon / hammerspoon

Staggeringly powerful macOS desktop automation with Lua
http://www.hammerspoon.org
MIT License
12.13k stars 586 forks source link

Security vulnerability in `hs.execute` function, susceptible to shell injection attacks #3701

Open SoraTakai opened 1 month ago

SoraTakai commented 1 month ago

hs.execute is vulnerable to shell injection attacks due to the lack of input escaping.

The most problematic line is

f = io.popen(os.getenv("SHELL")..[[ -l -i -c "]]..command..[["]], 'r')

when with_user_env is set to true.

If, e.g., someone sent a request to execute the following to Hammerspoon:

hs.execute('echo hi"; rm -rf /; echo "', true)

It would result in the command execution of:

/bin/bash -l -i -c "echo hi"; rm -rf /; echo ""

(Assuming $SHELL envvar is set to /bin/bash.)

Here is the proposed fix:

hs.execute = function(command, user_env)
  local f

  -- Function to safely quote the command for shell execution
  local function shellquote(str)
    return "'" .. str:gsub("'", "'\\''") .. "'"
  end

  local shell = os.getenv("SHELL") or "/bin/bash"

  if user_env then
    f = io.popen(shell .. " -l -i -c " .. shellquote(command), 'r')
  else
    -- io.popen is problematic to begin with, and this is a safer solution
    f = io.popen(shell .. " -c " .. shellquote(command), 'r')
  end

  local s = f:read('*a')
  local status, exit_type, rc = f:close()
  return s, status, exit_type, rc
end
cmsj commented 1 month ago

I'm somewhat tempted to argue that this isn't really a problem because users should be in control of their Hammerspoon config, but I think your proposed fix is more correct in terms of the shell commands being executed the way people will expect.

Thanks for the detailed report!