Conditionally harden methods

docs/config.md

@@ -34,6 +34,14 @@ set it via `Aikido::Zen.config.token = <token>`.
 
 **NOTE**: Never commit your token to the source code repository in plain text.
 
+## Hardened mode
+
+Zen hardens methods, restricting dangerous undocumented behavior to improve
+security and performance.
+
+To disable method hardening, set `AIKIDO_HARDEN=false` in your environment,
+or set `Aikido::Zen.config.harden = false`.
+
 ## Logger
 
 Zen logs to standard output by default. You can change this by changing the

lib/aikido/zen/config.rb

@@ -153,6 +153,11 @@ class Config
     #   allow known hosts that should be able to resolve to the IMDS service.
     attr_accessor :imds_allowed_hosts
 
+    # @return [Boolean] whether Aikido Zen should harden methods where possible.
+    #   Defaults to true. Can be set through AIKIDO_HARDEN environment variable.
+    attr_accessor :harden
+    alias_method :harden?, :harden
+
     def initialize
       self.disabled = read_boolean_from_env(ENV.fetch("AIKIDO_DISABLED", false))
       self.blocking_mode = read_boolean_from_env(ENV.fetch("AIKIDO_BLOCK", false))
@@ -185,6 +190,7 @@ def initialize
       self.api_schema_collection_max_properties = 20
       self.stored_ssrf = read_boolean_from_env(ENV.fetch("AIKIDO_FEATURE_STORED_SSRF", true))
       self.imds_allowed_hosts = ["metadata.google.internal", "metadata.goog"]
+      self.harden = read_boolean_from_env(ENV.fetch("AIKIDO_HARDEN", true))
     end
 
     # Set the base URL for API requests.

lib/aikido/zen/context.rb

@@ -99,13 +99,45 @@ def extract_payloads_from(data, source_type, prefix = nil)
           extract_payloads_from(value, source_type, [prefix, key].compact.join("."))
         end
       elsif data.respond_to?(:to_ary)
-        data.to_ary.flat_map.with_index do |value, index|
+        array = data.to_ary
+        return array if array.empty?
+
+        payloads = array.flat_map.with_index do |value, index|
           extract_payloads_from(value, source_type, [prefix, index].compact.join("."))
         end
+
+        unless Aikido::Zen.config.harden?
+          # Special case for File.join given a possibly nested array of strings,
+          # as might occur when a query parameter is an array.
+          begin
+            string = File.join__internal_for_aikido_zen(*array)
+            if unsafe_path?(string)
+              payloads << Payload.new(string, source_type, [prefix, "__File.join__"].compact.join("."))
+            end
+          rescue
+            # Could not create special payload for File.join.
+          end
+        end
+
+        payloads
       else
         [Payload.new(data, source_type, prefix.to_s)]
       end
     end
+
+    def unsafe_path?(filepath)
+      normalized_filepath = Pathname.new(filepath).cleanpath.to_s.downcase
+
+      Scanners::PathTraversal::DANGEROUS_PATH_PARTS.each do |dangerous_path_part|
+        return true if normalized_filepath.include?(dangerous_path_part)
+      end
+
+      Scanners::PathTraversal::DANGEROUS_PATH_STARTS.each do |dangerous_path_start|
+        return true if normalized_filepath.start_with?(dangerous_path_start)
+      end
+
+      false
+    end
   end
 end
 

lib/aikido/zen/sinks/file.rb

@@ -82,38 +82,40 @@ def self.load_sinks!
           end
 
           def join(*args, **kwargs, &blk)
-            # IMPORTANT: THE BEHAVIOR OF THIS METHOD IS CHANGED!
-            #
-            # File.join has undocumented behavior:
-            #
-            # File.join recursively joins nested string arrays.
-            #
-            # This prevents path traversal detection when an array originates
-            # from user input that was assumed to be a string.
-            #
-            # This undocumented behavior has been restricted to support path
-            # traversal detection.
-            #
-            # File.join no longer joins nested string arrays, but still accepts
-            # a single string array argument.
-
-            # File.join is often incorrectly called with a single array argument.
-            #
-            # i.e.
-            #
-            # File.join(["prefix", "filename"])
-            #
-            # This is considered acceptable.
-            #
-            # Calling File.join with a single string argument returns the string
-            # argument itself, having no practical effect. Therefore, it can be
-            # presumed that if File.join is called with a single array argument
-            # then this was its intended usage, and the array did not originate
-            # from user input that was assumed to be a string.
-            strings = args
-            strings = args.first if args.size == 1 && args.first.is_a?(Array)
-            strings.each do |string|
-              raise TypeError.new("Zen prevented implicit conversion of Array to String") if string.is_a?(Array)
+            if Aikido::Zen.config.harden?
+              # IMPORTANT: THE BEHAVIOR OF THIS METHOD IS CHANGED!
+              #
+              # File.join has undocumented behavior:
+              #
+              # File.join recursively joins nested string arrays.
+              #
+              # This prevents path traversal detection when an array originates
+              # from user input that was assumed to be a string.
+              #
+              # This undocumented behavior has been restricted to support path
+              # traversal detection.
+              #
+              # File.join no longer joins nested string arrays, but still accepts
+              # a single string array argument.
+
+              # File.join is often incorrectly called with a single array argument.
+              #
+              # i.e.
+              #
+              # File.join(["prefix", "filename"])
+              #
+              # This is considered acceptable.
+              #
+              # Calling File.join with a single string argument returns the string
+              # argument itself, having no practical effect. Therefore, it can be
+              # presumed that if File.join is called with a single array argument
+              # then this was its intended usage, and the array did not originate
+              # from user input that was assumed to be a string.
+              strings = args
+              strings = args.first if args.size == 1 && args.first.is_a?(Array)
+              strings.each do |string|
+                raise TypeError.new("Zen prevented implicit conversion of Array to String in hardened method. Visit https://github.com/AikidoSec/firewall-ruby for more information.") if string.is_a?(Array)
+              end
             end
 
             result = join__internal_for_aikido_zen(*args, **kwargs, &blk)

test/aikido/zen/context_test.rb

@@ -77,6 +77,32 @@ module GenericTests
       assert_includes context.payloads, stub_payload(:body, "7", "y.w.j")
     end
 
+    test "relative path arrays in complex parameter structures are joined for File.join" do
+      context = build_context_for("/path?path1[]=..&path1[]=..&path1[]=..&path1[]=etc&path1[]=passwd", {
+        method: "POST",
+        params: {path2: ["..", "..", "..", "etc", "passwd"]}
+      })
+
+      assert_includes context.payloads, stub_payload(:query, "../../../etc/passwd", "path1.__File.join__")
+
+      assert_includes context.payloads, stub_payload(:body, "../../../etc/passwd", "path2.__File.join__")
+    end
+
+    test "absolute path arrays in complex parameter structures are joined for File.join" do
+      context = build_context_for("/path?path1[]=&path1[]=etc&path1[]=passwd&path2[]=/&path2[]=etc&path2[]=passwd&&path3[]=/etc&path3[]=passwd", {
+        method: "POST",
+        params: {path4: ["/", "etc", "passwd"], path5: ["", "etc", "passwd"], path6: ["/etc", "passwd"]}
+      })
+
+      assert_includes context.payloads, stub_payload(:query, "/etc/passwd", "path1.__File.join__")
+      assert_includes context.payloads, stub_payload(:query, "/etc/passwd", "path2.__File.join__")
+      assert_includes context.payloads, stub_payload(:query, "/etc/passwd", "path3.__File.join__")
+
+      assert_includes context.payloads, stub_payload(:body, "/etc/passwd", "path4.__File.join__")
+      assert_includes context.payloads, stub_payload(:body, "/etc/passwd", "path5.__File.join__")
+      assert_includes context.payloads, stub_payload(:body, "/etc/passwd", "path6.__File.join__")
+    end
+
     test "#protection_disabled? allows requests from allowed IPs" do
       settings = Aikido::Zen.runtime_settings
       settings.update_from_json({
@@ -103,6 +129,7 @@ class RackRequestTest < ActiveSupport::TestCase
 
     setup do
       Aikido::Zen.config.request_builder = Aikido::Zen::Context::RACK_REQUEST_BUILDER
+      Aikido::Zen.config.harden = false
     end
 
     def build_context_for(path, env = {})
@@ -196,6 +223,7 @@ class RailsRequestTest < ActiveSupport::TestCase
 
     setup do
       Aikido::Zen.config.request_builder = Aikido::Zen::Context::RAILS_REQUEST_BUILDER
+      Aikido::Zen.config.harden = false
     end
 
     def env_for(path, env = {})
@@ -238,6 +266,28 @@ def stub_payload(source, value, path)
       assert_includes context.payloads, stub_payload(:body, 3, "array.2")
     end
 
+    test "relative path arrays in body payloads read from JSON-encoded bodies are joined for File.join" do
+      context = build_context_for("/example", {
+        :method => "POST",
+        :input => %({"path":["..", "..", "..", "etc", "passwd"]}),
+        "CONTENT_TYPE" => "application/json"
+      })
+
+      assert_includes context.payloads, stub_payload(:body, "../../../etc/passwd", "path.__File.join__")
+    end
+
+    test "absolute path arrays in body payloads read from JSON-encoded bodies are joined for File.join" do
+      context = build_context_for("/example", {
+        :method => "POST",
+        :input => %({"path1": ["/", "etc", "passwd"], "path2": ["", "etc", "passwd"], "path3": ["/etc", "passwd"]}),
+        "CONTENT_TYPE" => "application/json"
+      })
+
+      assert_includes context.payloads, stub_payload(:body, "/etc/passwd", "path1.__File.join__")
+      assert_includes context.payloads, stub_payload(:body, "/etc/passwd", "path2.__File.join__")
+      assert_includes context.payloads, stub_payload(:body, "/etc/passwd", "path3.__File.join__")
+    end
+
     test "route payloads are read from the data extracted by the router" do
       router = MockedRailsRouter.build do
         match "/example/:resource(/:id)(.:format)",