CVE-2022-24803: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: Asciidoctor-include-ext is Asciidoctor’s standard include processor reimplemented as an extension. Versions prior to 0.4.0, when used to render user-supplied input in AsciiDoc markup, may allow an attacker to execute arbitrary system commands on the host operating system. This attack is possible even when `allow-uri-read` is disabled! The problem has been patched in the referenced commits.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2022-04-01T00:15Z
Repository: https://github.com/jirutka/asciidoctor-include-ext
Language: Ruby
Files changed: lib/asciidoctor/include_ext/include_processor.rb, spec/integration_spec.rb
Commit date: 2022-03-30 00:50:25 +0200
Commit message: Fix command injection vulnerability

Reported by Joern Schneeweisz from GitLab GmbH:

> During an internal assessment on some GitLab code I came across a way
> to execute arbitrary commands in your asciidoctor-include-ext Gem.
>
> The following adoc snippet demonstrates the issue:
>
> ```
> :app-name: |id # + \
> http://test.com
>
> include::{app-name}[]
> ```
>
> It uses a linebreak to bypass the `target_uri` check here
> https://github.com/jirutka/asciidoctor-include-ext/blob/master/lib/asciidoctor/include_ext/include_processor.rb#L97
> and feed a command with the `|` prefix to open/IO.foreach.
>
> You can verify this with the above snippet by rendering it like this
>
> ```
> asciidoctor -r asciidoctor-include-ext  -a allow-uri-read home.asciidoc
> ```

See-Also: https://sakurity.com/blog/2015/02/28/openuri.html

Diff: diff --git a/lib/asciidoctor/include_ext/include_processor.rb b/lib/asciidoctor/include_ext/include_processor.rb
index 1a4c994..5bd52d0 100644
--- a/lib/asciidoctor/include_ext/include_processor.rb
+++ b/lib/asciidoctor/include_ext/include_processor.rb
@@ -1,8 +1,9 @@
 # frozen_string_literal: true
 require 'logger'
 require 'open-uri'
+require 'uri'
 
 require 'asciidoctor/include_ext/version'
 require 'asciidoctor/include_ext/reader_ext'
 require 'asciidoctor/include_ext/lineno_lines_selector'
 require 'asciidoctor/include_ext/logging'
@@ -84,19 +85,19 @@ module Asciidoctor::IncludeExt
     def include_allowed?(target, reader)
       doc = reader.document
 
       return false if doc.safe >= ::Asciidoctor::SafeMode::SECURE
       return false if doc.attributes.fetch('max-include-depth', 64).to_i < 1
-      return false if target_uri?(target) && !doc.attributes.key?('allow-uri-read')
+      return false if target_http?(target) && !doc.attributes.key?('allow-uri-read')
       true
     end
 
     # @param target (see #process)
     # @param reader (see #process)
     # @return [String, nil] file path or URI of the *target*, or `nil` if not found.
     def resolve_target_path(target, reader)
-      return target if target_uri? target
+      return target if target_http? target
 
       # Include file is resolved relative to dir of the current include,
       # or base_dir if within original docfile.
       path = reader.document.normalize_system_path(target, reader.dir, nil,
                                                    target_name: 'include file')
@@ -104,20 +105,20 @@ module Asciidoctor::IncludeExt
     end
 
     # Reads the specified file as individual lines, filters them using the
     # *selector* (if provided) and returns those lines in an array.
     #
-    # @param filename [String] path of the file to be read.
+    # @param path [String] URL or path of the file to be read.
     # @param selector [#to_proc, nil] predicate to filter lines that should be
     #   included in the output. It must accept two arguments: line and
     #   the line number. If `nil` is given, all lines are passed.
     # @return [Array<String>] an array of read lines.
-    def read_lines(filename, selector)
+    def read_lines(path, selector)
       if selector
-        IO.foreach(filename).select.with_index(1, &selector)
+        IO.foreach(path).select.with_index(1, &selector)
       else
-        URI.open(filename, &:read)
+        URI.open(path, &:read)
       end
     end
 
     # Finds and initializes a lines selector that can handle the specified include.
     #
@@ -140,11 +141,15 @@ module Asciidoctor::IncludeExt
     end
 
     private
 
     # @param target (see #process)
-    # @return [Boolean] `true` if the *target* is an URI, `false` otherwise.
-    def target_uri?(target)
-      ::Asciidoctor::Helpers.uriish?(target)
+    # @return [Boolean] `true` if the *target* is a valid HTTP(S) URI, `false` otherwise.
+    def target_http?(target)
+      # First do a fast test, then try to parse it.
+      target.downcase.start_with?('http://', 'https://') \
+        && URI.parse(target).is_a?(URI::HTTP)
+    rescue URI::InvalidURIError
+      false
     end
   end
 end
diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb
index d432453..ba13a94 100644
--- a/spec/integration_spec.rb
+++ b/spec/integration_spec.rb
@@ -136,10 +136,24 @@ describe 'Integration tests' do
       ADOC
 
       should match /let s = SS.empty;;/
       should_not match /(?:tag|end)::snippet\[\]/
     end
+
+    it 'does not allow execution of system command when allow-uri-read is set' do
+      options.merge!(attributes: { 'allow-uri-read' => '' })
+      given <<~ADOC
+        :app-name: |cat LICENSE # + \\
+        http://test.com
+
+        include::{app-name}[]
+      ADOC
+
+      should match /unresolved/i
+      should_not match /The MIT License/
+    end
+
   end
 
 
   #----------  Helpers  ----------
 

Vulnerable code:       return false if target_uri?(target) && !doc.attributes.key?('allow-uri-read')
      return target if target_uri? target
    # @param filename [String] path of the file to be read.
    def read_lines(filename, selector)
        IO.foreach(filename).select.with_index(1, &selector)
        URI.open(filename, &:read)
    # @return [Boolean] `true` if the *target* is an URI, `false` otherwise.
    def target_uri?(target)
      ::Asciidoctor::Helpers.uriish?(target)
Fixed code: require 'uri'
      return false if target_http?(target) && !doc.attributes.key?('allow-uri-read')
      return target if target_http? target
    # @param path [String] URL or path of the file to be read.
    def read_lines(path, selector)
        IO.foreach(path).select.with_index(1, &selector)
        URI.open(path, &:read)
    # @return [Boolean] `true` if the *target* is a valid HTTP(S) URI, `false` otherwise.
    def target_http?(target)
      # First do a fast test, then try to parse it.
      target.downcase.start_with?('http://', 'https://') \
        && URI.parse(target).is_a?(URI::HTTP)
    rescue URI::InvalidURIError
      false

    it 'does not allow execution of system command when allow-uri-read is set' do
      options.merge!(attributes: { 'allow-uri-read' => '' })
      given <<~ADOC
        :app-name: |cat LICENSE # + \\
        http://test.com

        include::{app-name}[]
      ADOC

      should match /unresolved/i
      should_not match /The MIT License/
    end