[ruby-core:113864] [Ruby master Bug#19723] [RFC] Deprecate/disallow passing `"|command..." values to open-uri's URI.open() method

Issue #19723 has been reported by postmodern (Hal Brodigan). ---------------------------------------- Bug #19723: [RFC] Deprecate/disallow passing `"|command..." values to open-uri's URI.open() method https://bugs.ruby-lang.org/issues/19723 * Author: postmodern (Hal Brodigan) * Status: Open * Priority: Normal * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- Due to `Kernel.open()` supporting opening pipe-commands (ex: `"|command-here..."`) this has led to multiple [1] security [2] vulnerabilities [3], where malicious user-input eventually is passed to `Kernel.open()`. One of the code-paths that malicious user-input can reach `Kernel.open()` is via open-uri's `URI.open()` method. RuboCop even recommends avoiding using `URI.open()` in favor of `uri = URI.parse(...); uri.open` to avoid accidentally opening malicious `"|command..."` inputs. I propose that `URI.open()` should not accept pipe-commands, as they are neither URIs nor files. One could even argue that `URI.open()` should only accept URIs and never fallback to `Kernel.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 #19723 has been updated by mdalessio (Mike Dalessio). I think we should merge this discussion into #19630 since the behavior you wish to deprecate comes from `Kernel#open` (called by `URI.open` in the fall-through case). If #19630 is accepted, the naive implementation proposed at https://github.com/ruby/ruby/pull/7915 would _also_ deprecate this behavior in `URI.open`. ---------------------------------------- Feature #19723: [RFC] Deprecate/disallow passing `"|command..." values to open-uri's URI.open() method https://bugs.ruby-lang.org/issues/19723#change-103520 * Author: postmodern (Hal Brodigan) * Status: Open * Priority: Normal ---------------------------------------- Due to `Kernel.open()` supporting opening pipe-commands (ex: `"|command-here..."`) this has led to multiple [1] security [2] vulnerabilities [3], where malicious user-input eventually is passed to `Kernel.open()`. One of the code-paths that malicious user-input can reach `Kernel.open()` is via open-uri's `URI.open()` method. RuboCop even recommends avoiding using `URI.open()` in favor of `uri = URI.parse(...); uri.open` to avoid accidentally opening malicious `"|command..."` inputs. I propose that `URI.open()` should not accept pipe-commands, as they are neither URIs nor files. One could even argue that `URI.open()` should only accept URIs and never fallback to `Kernel.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 #19723 has been updated by postmodern (Hal Brodigan). mdalessio (Mike Dalessio) wrote in #note-2:
I think we should merge this discussion into #19630 since the behavior you wish to deprecate comes from `Kernel#open` (called by `URI.open` in the fall-through case).
If #19630 is accepted, the naive implementation proposed at https://github.com/ruby/ruby/pull/7915 would _also_ deprecate this behavior in `URI.open`.
This could be done before #19630 by changing `URI.open` to either fallback to `File.open` or not fallback to `open` at all. We could preemptively close this vulnerable code path before Ruby 4.0, since `URI.open` implies that it opens URIs and only URIs. ---------------------------------------- Feature #19723: [RFC] Deprecate/disallow passing `"|command..." values to open-uri's URI.open() method https://bugs.ruby-lang.org/issues/19723#change-103522 * Author: postmodern (Hal Brodigan) * Status: Open * Priority: Normal ---------------------------------------- Due to `Kernel.open()` supporting opening pipe-commands (ex: `"|command-here..."`) this has led to multiple [1] security [2] vulnerabilities [3], where malicious user-input eventually is passed to `Kernel.open()`. One of the code-paths that malicious user-input can reach `Kernel.open()` is via open-uri's `URI.open()` method. RuboCop even recommends avoiding using `URI.open()` in favor of `uri = URI.parse(...); uri.open` to avoid accidentally opening malicious `"|command..."` inputs. I propose that `URI.open()` should not accept pipe-commands, as they are neither URIs nor files. One could even argue that `URI.open()` should only accept URIs and never fallback to `Kernel.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 #19723 has been updated by hsbt (Hiroshi SHIBATA). Status changed from Open to Closed @akr and @matz accepted this deprecation at [Misc #19722: DevMeeting-2023-07-13](https://bugs.ruby-lang.org/issues/19722) I'll merge this into https://bugs.ruby-lang.org/issues/19630 ---------------------------------------- Feature #19723: [RFC] Deprecate/disallow passing `"|command..." values to open-uri's URI.open() method https://bugs.ruby-lang.org/issues/19723#change-103864 * Author: postmodern (Hal Brodigan) * Status: Closed * Priority: Normal ---------------------------------------- Due to `Kernel.open()` supporting opening pipe-commands (ex: `"|command-here..."`) this has led to multiple [1] security [2] vulnerabilities [3], where malicious user-input eventually is passed to `Kernel.open()`. One of the code-paths that malicious user-input can reach `Kernel.open()` is via open-uri's `URI.open()` method. RuboCop even recommends avoiding using `URI.open()` in favor of `uri = URI.parse(...); uri.open` to avoid accidentally opening malicious `"|command..."` inputs. I propose that `URI.open()` should not accept pipe-commands, as they are neither URIs nor files. One could even argue that `URI.open()` should only accept URIs and never fallback to `Kernel.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/
participants (3)
-
hsbt (Hiroshi SHIBATA)
-
mdalessio (Mike Dalessio)
-
postmodern (Hal Brodigan)