Issue #19630 has been updated by postmodern (Hal Brodigan). A more complete list of the CVEs related to `Kernel.open`: * CVE-2017-17405 (ruby, net-ftp) * CVE-2017-17790 (ruby, resolv) * CVE-2019-10780 (bibtex-ruby) * CVE-2021-21289 (mechanize) * CVE-2019-5477 (nokogiri) * CVE-2021-31799 (rdoc) * CVE-2019-5477 (rexical) ---------------------------------------- Feature #19630: [RFC] Deprecate `Kernel.open("|command-here")` due to frequent security issues https://bugs.ruby-lang.org/issues/19630#change-102980 * Author: postmodern (Hal Brodigan) * Status: Open * Priority: Normal ---------------------------------------- `Kernel.open()` is the source of numerous [1] security [2] issues [3], due to the fact that it can be used to execute commands if given a String argument of the form `"|command-here"`. However, in most uses of `Kernel.open()` the developer appears to either want to open a local file, or if 'open-uri' was explicitly required open a remote URI. We should deprecate calling `Kernel.open()` with a `"|command-here"` style arguments, with a warning message instructing the developer to use `IO.popen()` instead. Eventually, support for `Kernel.open("|command-here")` could be removed completely, in favor of having the developer explicitly call `IO.popen()` or `URI.open()`. [1]: https://45w1nkv.medium.com/ruby-code-vulnerability-analysis-confirmsnssubscr... [2]: https://bishopfox.com/blog/ruby-vulnerabilities-exploits [3]: https://blog.heroku.com/identifying-ruby-ftp-cve -- https://bugs.ruby-lang.org/

Issue #19630 has been updated by hsbt (Hiroshi SHIBATA). This proposal make sense to me. But I'm not sure how impact existing code for this incompatibility. Do you have any deprecated process for this? ---------------------------------------- Feature #19630: [RFC] Deprecate `Kernel.open("|command-here")` due to frequent security issues https://bugs.ruby-lang.org/issues/19630#change-103254 * Author: postmodern (Hal Brodigan) * Status: Open * Priority: Normal ---------------------------------------- `Kernel.open()` is the source of numerous [1] security [2] issues [3], due to the fact that it can be used to execute commands if given a String argument of the form `"|command-here"`. However, in most uses of `Kernel.open()` the developer appears to either want to open a local file, or if 'open-uri' was explicitly required open a remote URI. We should deprecate calling `Kernel.open()` with a `"|command-here"` style arguments, with a warning message instructing the developer to use `IO.popen()` instead. Eventually, support for `Kernel.open("|command-here")` could be removed completely, in favor of having the developer explicitly call `IO.popen()` or `URI.open()`. [1]: https://45w1nkv.medium.com/ruby-code-vulnerability-analysis-confirmsnssubscr... [2]: https://bishopfox.com/blog/ruby-vulnerabilities-exploits [3]: https://blog.heroku.com/identifying-ruby-ftp-cve -- https://bugs.ruby-lang.org/

Issue #19630 has been updated by byroot (Jean Boussier).

for now just print a deprecation warning.

My worry is that since deprecation warnings are disabled by default, many people might not notice. Recent examples show that things like `File.exists?` was deprecated for a decade, and some people were still surprised by its removal. I know it's a distinct issue, but it impacts this one. ---------------------------------------- Feature #19630: [RFC] Deprecate `Kernel.open("|command-here")` due to frequent security issues https://bugs.ruby-lang.org/issues/19630#change-103437 * Author: postmodern (Hal Brodigan) * Status: Open * Priority: Normal ---------------------------------------- `Kernel.open()` is the source of numerous [1] security [2] issues [3], due to the fact that it can be used to execute commands if given a String argument of the form `"|command-here"`. However, in most uses of `Kernel.open()` the developer appears to either want to open a local file, or if 'open-uri' was explicitly required open a remote URI. We should deprecate calling `Kernel.open()` with a `"|command-here"` style arguments, with a warning message instructing the developer to use `IO.popen()` instead. Eventually, support for `Kernel.open("|command-here")` could be removed completely, in favor of having the developer explicitly call `IO.popen()` or `URI.open()`. [1]: https://45w1nkv.medium.com/ruby-code-vulnerability-analysis-confirmsnssubscr... [2]: https://bishopfox.com/blog/ruby-vulnerabilities-exploits [3]: https://blog.heroku.com/identifying-ruby-ftp-cve -- https://bugs.ruby-lang.org/

Issue #19630 has been updated by postmodern (Hal Brodigan). @mdalessio (Mike Dalessio) wrote in #note-5:

@hsbt Because this functionality has existed in Ruby for such a long time, maybe we should target the next major release for removal of this functionality, and for now just print a deprecation warning.

@postmodern Just to clarify, you're only suggesting deprecating this in `Kernel#open`. It's also possible for commands to be injected into:

- IO.binread - IO.foreach - IO.readlines - IO.read - IO.write

but my understanding is that you're proposing to leave these methods alone, is that correct?

If noone has objections, I'll create a pull request so we have something concrete to discuss.

I was unaware that these methods can accept `|command` style inputs. Based on the stdlib documentation, the first argument is called `name` and the examples show reading from `testfile`, which implies to me they should only read from files. We could at first deprecate `Kernel.open` and see how much impact it has on users complaining about deprecation warnings, or we could start with the other `IO` methods? ---------------------------------------- Feature #19630: [RFC] Deprecate `Kernel.open("|command-here")` due to frequent security issues https://bugs.ruby-lang.org/issues/19630#change-103450 * Author: postmodern (Hal Brodigan) * Status: Open * Priority: Normal ---------------------------------------- `Kernel.open()` is the source of numerous [1] security [2] issues [3], due to the fact that it can be used to execute commands if given a String argument of the form `"|command-here"`. However, in most uses of `Kernel.open()` the developer appears to either want to open a local file, or if 'open-uri' was explicitly required open a remote URI. We should deprecate calling `Kernel.open()` with a `"|command-here"` style arguments, with a warning message instructing the developer to use `IO.popen()` instead. Eventually, support for `Kernel.open("|command-here")` could be removed completely, in favor of having the developer explicitly call `IO.popen()` or `URI.open()`. [1]: https://45w1nkv.medium.com/ruby-code-vulnerability-analysis-confirmsnssubscr... [2]: https://bishopfox.com/blog/ruby-vulnerabilities-exploits [3]: https://blog.heroku.com/identifying-ruby-ftp-cve -- https://bugs.ruby-lang.org/

Issue #19630 has been updated by nobu (Nobuyoshi Nakada). postmodern (Hal Brodigan) wrote in #note-8:

I was unaware that these methods can accept `|command` style inputs. Based on the stdlib documentation, the first argument is called `name` and the examples show reading from `testfile`, which implies to me they should only read from files. We could at first deprecate `Kernel.open` and see how much impact it has on users complaining about deprecation warnings, or we could start with the other `IO` methods?

I'm against deprecating pipe in `IO` methods. It is intentionally kept quiet, unlike `File`. ---------------------------------------- Feature #19630: [RFC] Deprecate `Kernel.open("|command-here")` due to frequent security issues https://bugs.ruby-lang.org/issues/19630#change-103503 * Author: postmodern (Hal Brodigan) * Status: Open * Priority: Normal ---------------------------------------- `Kernel.open()` is the source of numerous [1] security [2] issues [3], due to the fact that it can be used to execute commands if given a String argument of the form `"|command-here"`. However, in most uses of `Kernel.open()` the developer appears to either want to open a local file, or if 'open-uri' was explicitly required open a remote URI. We should deprecate calling `Kernel.open()` with a `"|command-here"` style arguments, with a warning message instructing the developer to use `IO.popen()` instead. Eventually, support for `Kernel.open("|command-here")` could be removed completely, in favor of having the developer explicitly call `IO.popen()` or `URI.open()`. [1]: https://45w1nkv.medium.com/ruby-code-vulnerability-analysis-confirmsnssubscr... [2]: https://bishopfox.com/blog/ruby-vulnerabilities-exploits [3]: https://blog.heroku.com/identifying-ruby-ftp-cve -- https://bugs.ruby-lang.org/

Issue #19630 has been updated by byroot (Jean Boussier).

is there any objection

Not from me, we should add this ticket to the next dev meeting. However I feel like other IO methods (`IO.binread`, etc) should do the same otherwise it's a bit inconsistent. ---------------------------------------- Feature #19630: [RFC] Deprecate `Kernel.open("|command-here")` due to frequent security issues https://bugs.ruby-lang.org/issues/19630#change-103534 * Author: postmodern (Hal Brodigan) * Status: Open * Priority: Normal ---------------------------------------- `Kernel.open()` is the source of numerous [1] security [2] issues [3], due to the fact that it can be used to execute commands if given a String argument of the form `"|command-here"`. However, in most uses of `Kernel.open()` the developer appears to either want to open a local file, or if 'open-uri' was explicitly required open a remote URI. We should deprecate calling `Kernel.open()` with a `"|command-here"` style arguments, with a warning message instructing the developer to use `IO.popen()` instead. Eventually, support for `Kernel.open("|command-here")` could be removed completely, in favor of having the developer explicitly call `IO.popen()` or `URI.open()`. [1]: https://45w1nkv.medium.com/ruby-code-vulnerability-analysis-confirmsnssubscr... [2]: https://bishopfox.com/blog/ruby-vulnerabilities-exploits [3]: https://blog.heroku.com/identifying-ruby-ftp-cve -- https://bugs.ruby-lang.org/

Issue #19630 has been updated by hsbt (Hiroshi SHIBATA). @mdalessio Could you also deprecate the following methods in your pull request? ``` IO.binread IO.foreach IO.readlines IO.read IO.write ``` ---------------------------------------- Feature #19630: [RFC] Deprecate `Kernel.open("|command-here")` due to frequent security issues https://bugs.ruby-lang.org/issues/19630#change-103860 * Author: postmodern (Hal Brodigan) * Status: Open * Priority: Normal ---------------------------------------- `Kernel.open()` is the source of numerous [1] security [2] issues [3], due to the fact that it can be used to execute commands if given a String argument of the form `"|command-here"`. However, in most uses of `Kernel.open()` the developer appears to either want to open a local file, or if 'open-uri' was explicitly required open a remote URI. We should deprecate calling `Kernel.open()` with a `"|command-here"` style arguments, with a warning message instructing the developer to use `IO.popen()` instead. Eventually, support for `Kernel.open("|command-here")` could be removed completely, in favor of having the developer explicitly call `IO.popen()` or `URI.open()`. [1]: https://45w1nkv.medium.com/ruby-code-vulnerability-analysis-confirmsnssubscr... [2]: https://bishopfox.com/blog/ruby-vulnerabilities-exploits [3]: https://blog.heroku.com/identifying-ruby-ftp-cve -- https://bugs.ruby-lang.org/

Issue #19630 has been updated by mdalessio (Mike Dalessio). @hsbt Do you think I should also deprecate pipe commends in `URI.open` as suggested in https://bugs.ruby-lang.org/issues/19723 ? It seems like @matz may be encouraging this by saying "all open methods" above. ---------------------------------------- Feature #19630: [RFC] Deprecate `Kernel.open("|command-here")` due to frequent security issues https://bugs.ruby-lang.org/issues/19630#change-103863 * Author: postmodern (Hal Brodigan) * Status: Open * Priority: Normal ---------------------------------------- `Kernel.open()` is the source of numerous [1] security [2] issues [3], due to the fact that it can be used to execute commands if given a String argument of the form `"|command-here"`. However, in most uses of `Kernel.open()` the developer appears to either want to open a local file, or if 'open-uri' was explicitly required open a remote URI. We should deprecate calling `Kernel.open()` with a `"|command-here"` style arguments, with a warning message instructing the developer to use `IO.popen()` instead. Eventually, support for `Kernel.open("|command-here")` could be removed completely, in favor of having the developer explicitly call `IO.popen()` or `URI.open()`. [1]: https://45w1nkv.medium.com/ruby-code-vulnerability-analysis-confirmsnssubscr... [2]: https://bishopfox.com/blog/ruby-vulnerabilities-exploits [3]: https://blog.heroku.com/identifying-ruby-ftp-cve -- https://bugs.ruby-lang.org/

Issue #19630 has been updated by ko1 (Koichi Sasada). memo:

rb_warn_deprecated_to_remove("4.0", "Calling Kernel#open with a leading '|'", "IO.popen");

* warning until 4.0 * delete at 4.0 ---------------------------------------- Feature #19630: [RFC] Deprecate `Kernel.open("|command-here")` due to frequent security issues https://bugs.ruby-lang.org/issues/19630#change-103866 * Author: postmodern (Hal Brodigan) * Status: Open * Priority: Normal ---------------------------------------- `Kernel.open()` is the source of numerous [1] security [2] issues [3], due to the fact that it can be used to execute commands if given a String argument of the form `"|command-here"`. However, in most uses of `Kernel.open()` the developer appears to either want to open a local file, or if 'open-uri' was explicitly required open a remote URI. We should deprecate calling `Kernel.open()` with a `"|command-here"` style arguments, with a warning message instructing the developer to use `IO.popen()` instead. Eventually, support for `Kernel.open("|command-here")` could be removed completely, in favor of having the developer explicitly call `IO.popen()` or `URI.open()`. [1]: https://45w1nkv.medium.com/ruby-code-vulnerability-analysis-confirmsnssubscr... [2]: https://bishopfox.com/blog/ruby-vulnerabilities-exploits [3]: https://blog.heroku.com/identifying-ruby-ftp-cve -- https://bugs.ruby-lang.org/