Mitigate remote code execution attack vector

[yagudaev]

It would be really nice if we could limit the remote code execution attack when binding on 0.0.0.0.

This was a huge turn off to see this:

WARNING

WARNING!!: cypress-on-rails can execute arbitrary ruby code Please use with extra caution if starting your local server on 0.0.0.0 or running the gem on a hosted server

In the README, so much so that I opted to not use this gem and implemented some of the functionality myself in our app. But looking at this gem again, I like a lot of the things you have built and would love to help think about this more.

I'm especially cautious after seeing this article popup on hacker news https://news.ycombinator.com/item?id=20028108 the other day.

Combining those two things: remote code execution and accessing localhost:3000 from another website would make a killer way to deliver viruses and other malicious code.

Anyway, love to hear more details where that is in the code and brainstorm ideas how to get around the need for remote code execution. It should only allow execution of specific helper code and factories from cypress. Also maybe we need to fail or let the user set a flag to even be able to use this when binding on 0.0.0.0

[yagudaev]

Ok taking a quick look at the code I see where the problem is:

https://github.com/shakacode/cypress-on-rails/blob/master/lib/cypress_dev/command_executor.rb#L9

  class CommandExecutor
    def self.load(file,command_options = nil)
      load_cypress_helper
      file_data = File.read(file)
      eval file_data
    rescue => e
      logger.error("fail to execute #{file}: #{e.message}")
      logger.error(e.backtrace.join("\n"))
      raise e
    end

If I am reading this right, this allows for any file on the machine to be used to execute ruby code. Adding a restrictions here to only allow loading files from "#{project_path}/spec/cypress/rails_helpers" or similar would really restrict the attack vector.

As an attack, what I would do otherwise, would be to figure out where the users browser saves files on the system and make sure it caches my own malicious file and get this gem to try to execute this code. If I cannot do that, it will make the system much more secure.

[yagudaev]

Looking at it even further and discussing with colleagues really quickly.

Cypress.Commands.add('appCommands', function (body) {
  cy.request({
    method: 'POST',
    url: "/__cypress__/command",
    body: JSON.stringify(body),
    log: true,
    failOnStatusCode: true
  })
});

This here means that if you have your CORS on rails enabled to be say *, anybody can execute code remotely as they please. Which is really scary. We need to make sure those paths don't allow wildcard CORS requests.

[grantspeelman]

Just to add some background on the notice.
When I added the notice what I was really referring to is the eval support:

eval app_command

Kernel.eval(command_options) unless command_options.nil?

and cypress function

Cypress.Commands.add('appEval', function (code) {
  cy.app('eval', code)
});

This allows you to write code like this, which can be useful for once of things:

    cy.appEval('UserService.disable_callbacks')

which can easily be abused if someone can access your server.

Doing eval's like this was supported in the first versions of cypress-on-rails, so i wanted to continue supporting it even though I deleted the file and function for all projects I work on.

[grantspeelman]

Limiting files to only be loaded from the configured cypress_folder directory is a good idea i think to increasing the security.

[grantspeelman]

We could also look at doing something like a api token that we can be generated on install and is added to cypress.env.json which cypress can access and is easy enough for the gem to read server side to confirm the token on any request.

[yagudaev]

@grantspeelman thanks for the quick and thoughtful response and building the gem :).

Doing eval's like this was supported in the first versions of cypress-on-rails, so i wanted to continue supporting it even though I deleted the file and function for all projects I work on.

Eliminating the need to do eval would really simplify things. There are other features of cypress-on-rails that can be used to accomplish the same thing like scenarios and helpers.

We can deprecate it more slowly if you would like and turn it off by default on new installs. We can even allow the users to opt-in to using this feature at their own risk. Simply by leaving a comment in the code in config like:

CypressDev.configure do |config|
  # WARNING!!: cypress-on-rails can execute arbitrary ruby code if the code eval options is enabled. Please use with extra caution if starting your local server on 0.0.0.0, running the gem on a hosted server and with your CORS setup. For more details see: https://github.com/shakacode/cypress-on-rails/issues/13
  # config.enable_code_eval
end

A few years ago I had to secure a code eval for arbitrary business rules that code be added to an app. Think spreadsheet style, you add rules that effected your models. To accomplish that I used Distributed Ruby (drb). I started another ruby process with very restricted access (different user) to do the eval. I then sent the string to be evaluated to that less privileged process.

It was a lot of work. Took like 6 weeks to get it all dialled in.

Another idea is taking a look at other gems that have this problem. The top one would be Rails Web Console. They have a special permission for IPs that are used by the web console.

class Application < Rails::Application
  config.web_console.permissions = '192.168.0.100'
end

https://github.com/rails/web-console#configweb_consolepermissions

We can adopt the same idea for both CORS and IP.

Looking at it a web console evaluator a little deeper it uses a unique id for every repel request: https://github.com/rails/web-console/blob/master/lib/web_console/session.rb#L45. That's to protect against reply attacks. It also means the path to execute the request changes everytime making it harder to guess it.

We could also look at doing something like a api token that we can be generated on install and is added to cypress.env.json which cypress can access and is easy enough for the gem to read server side to confirm the token on any request.

This would help a lot.

We should also link from the warning to in the readme to this discussion here to let other contribute to it more easily.

======

My biggest question remains, can we drop the appEval and avoid a lot of this complexity?

[yagudaev]

@grantspeelman can you add a license to the project? #12 Pretty please? :)

I would like to try and use this gem, fork it and try spiking on some of the things we talked about above. I can't do that until we have a valid OSS license.

Added PR #14 to do just that

[justin808]

@yagudaev If you want a submit a pull request to tighten up the security, we'll get it reviewed ASAP.

@grantspeelman and I feel that the gem should not be installed such that it's never used on production, like:

https://github.com/BetterErrors/better_errors#security

[lorennorman]

I just read this issue to get caught up, but it's getting a little stale and I'm not sure where things stand. Is this still in consideration?

I happily deleted eval.rb and the appEval Cypress command in my app, but the File.read() and eval inside CommandExecutor are glaring at me. I don't see where this was addressed? I don't understand why this is necessary in the first place. The suggestion to lock down the File.read to only allow access to the Rails directories seems great to me.

Big thanks to everyone for working hard and caring! ❤️

[bmulholland]

The eval is required because the approach is to have files (in app_commands) that are executed. What about changing to a central switch statement that runs the necessary code? That would be more explicit, too: all commands would go through a central place.

Something like:

CypressOnRails.configure do |c|
  c.execute_commands = lambda do |args|
    case args.first
    when "appFactory"
      # load file, run method, whatever
    end
  end
end