calabash / calabash-android

Automated Functional testing for Android using cucumber
Other
1.68k stars 617 forks source link

GitHub Security Lab (GHSL) Vulnerability Report: GHSL-2022-095 #955

Open Kwstubbs opened 1 year ago

Kwstubbs commented 1 year ago

The GitHub Security Lab team has identified a potential security vulnerability in Calabash for Android.

We are committed to working with you to help resolve this issue. In this report you will find everything you need to effectively coordinate a resolution of this issue with the GHSL team.

If at any point you have concerns or questions about this process, please do not hesitate to reach out to us at securitylab@github.com (please include GHSL-2022-095 as a reference).

If you are NOT the correct point of contact for this report, please let us know!

Summary

The screenshot_embed method may lead to remote code execution if invoked with untrusted user-controlled data.

Product

Calabash for Android

Tested Version

v0.9.27 (latest)

Details

Issue: Unsafe shell command constructed from library input in screenshot. (GHSL-2022-095)

The operations.rb file unsafely constructs a shell string using various parameters, which can potentially leave clients of calabash vulnerable to command injection.

The library is not directly exploitable: the exploit requires that some client of the library calls the vulnerable method with user input. However, if unsafe input reaches the library method, then an attacker can execute arbitrary shell commands on the host machine.

Proof of Concept

Here is a PoC (relevant code copy-pasted into the PoC for ease of setup). The observed effect is that a new file pwned is created in the current working directory. The code crashes, but that only happens after the payload has executed.

# A snippet of the `screenshot_embed` and `screenshot` functions from `operations.rb`.
def screenshot_embed(options={:prefix => nil, :name => nil, :label => nil})
  path = screenshot(options)
  # ....
end
def screenshot(options={:prefix => nil, :name => nil})
  screenshot_count = 0 # Faking some variable
  serial = 0 # Faking some variable
  prefix = options[:prefix] || ENV['SCREENSHOT_PATH'] || ""
  name = options[:name]
  if name.nil?
    name = "screenshot"
  else
    if File.extname(name).downcase == ".png"
      name = name.split(".png")[0]
    end
  end
  # @@screenshot_count ||= 0
  path = "#{prefix}#{name}_#{screenshot_count}.png"
  if false # ENV["SCREENSHOT_VIA_USB"] == "false"
    begin
      res = http("/screenshot")
    rescue EOFError
      raise "Could not take screenshot. App is most likely not running anymore."
    end
    File.open(path, 'wb') do |f|
      f.write res
    end
  else
    screenshot_cmd = "java -jar \"#{File.join(File.dirname(__FILE__), 'lib', 'screenshotTaker.jar')}\" #{serial} \"#{path}\""
    # calabash_log screenshot_cmd
    raise "Could not take screenshot" unless system(screenshot_cmd)
  end
  # @@screenshot_count += 1
  path
end
# Execute the payload
screenshot_embed(:prefix => "`touch pwned`foo")

But there are many more vulnerable methods in operations.rb. (see these results).

Impact

This issue may lead to Remote Code Execution (RCE).

Remediation

Call system with an array instead of a string, preventing the latter from getting interpreted as a shell command.

Resources

CodeQL for Ruby - Unsafe shell command constructed from library input.

GitHub Security Advisories

We recommend you create a private GitHub Security Advisory for this finding. This also allows you to invite the GHSL team to collaborate and further discuss this finding in private before it is published.

Credit

This issue was discovered and reported by GHSL team member @erik-krogh (Erik Krogh Kristensen).

Contact

You can contact the GHSL team at securitylab@github.com, please include a reference to GHSL-2022-095 in any communication regarding this issue.

Disclosure Policy

This report is subject to our coordinated disclosure policy.