Add cop forbidding Kernel.open method call
Description of the proposal
According to our security guidelines, the method open or Kernel.open can be used to execute commands as well. For example, with a simple method like:
def open_url(uri)
open(uri)
end
Without the necessary validation in the uri variable, a malicious user could perform an attack by just setting the uri to |cat /etc/passwd > /usr/share/nginx/html/copied or any other kind. We'll be basically giving a console to the user.
Besides that, the Kernel.open method can be used to open URLs. This is discouraged in favor of Gitlab::HTTP because we don't have control over Kernel.open and we can't validate if, for example, the request is made to localhost.
I haven't found any reference in the codebase to this method so, yay!
Since version 0.53, Rubocop enables by default the cop Security/Open, which does exactly this, but there are several cases not covered. It behaves like this:
# Bad
open(foo)
open("|foo #{bar}")
# Good
open("foo")
open("|foo")
open("foo #{bar}")
Kernel.open("foo")
Kernel.open("|foo")
Kernel.open("foo#{bar}")
Kernel.open("|foo #{bar}")
Kernel.open(foo)
The cop we're proposing will be more restrictive and it will behave like:
# Good
open # without arguments
# Bad
open(foo)
open("|foo #{bar}")
open("|foo")
open("foo #{bar}")
Kernel.open("|foo")
Kernel.open("foo#{bar}")
Kernel.open("|foo #{bar}")
Kernel.open(foo)
False Positives
The default Security/Open can trigger some false positives:
class Foo
def open(file)
File.open(file)
end
def execute(file)
open(file) # false positive
end
end
The new cop will also have those false positives and also trigger some more in the scenarios commented above.
-
Mention the proposal in the next backend weekly call and the #backend channel to encourage contribution -
Proceed with the proposal once 50% of the maintainers have weighed in, and 80% of the votes are 👍 -
Once approved, mention it again in the next backend weekly call and the #backend channel