diff --git a/lib/aikido/zen/scanners/shell_injection/helpers.rb b/lib/aikido/zen/scanners/shell_injection/helpers.rb index 9160961e..3d47d040 100644 --- a/lib/aikido/zen/scanners/shell_injection/helpers.rb +++ b/lib/aikido/zen/scanners/shell_injection/helpers.rb @@ -22,53 +22,110 @@ module Helpers # @param command [string] # @param user_input [string] def self.is_safely_encapsulated(command, user_input) - segments = command.split(user_input) - - # The next condition is merely here to be compliant with what javascript does when splitting strings: - # From js doc https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/split - # > If separator appears at the beginning (or end) of the string, it still has the effect of splitting, - # > resulting in an empty (i.e. zero length) string appearing at the first (or last) position of - # > the returned array. - # This is necessary because this code is ported form the firewall-node code. - if user_input.length > 1 - if command.start_with? user_input - segments.unshift "" - end - - if command.end_with? user_input - segments << "" - end + # Return false if user input is not in the command + return true unless command.include?(user_input) + + # Find all occurrences of user_input in command + occurrences = [] + start_pos = 0 + while (pos = command.index(user_input, start_pos)) + occurrences << pos + start_pos = pos + 1 end - # Call the helper function to get current and next segments - get_current_and_next_segments(segments).all? do |segments_pair| - char_before_user_input = segments_pair[:current_segment][-1] - char_after_user_input = segments_pair[:next_segment][0] - - # Check if the character before is an escape character - is_escape_char = ESCAPE_CHARS.include?(char_before_user_input) + # Check if all occurrences are safely encapsulated + occurrences.all? do |occurrence_index| + is_occurrence_safely_encapsulated(command, user_input, occurrence_index) + end + end - unless is_escape_char - next false + # Check if a specific occurrence of user_input in command is safely encapsulated + # by parsing shell quote state from the beginning of the command + def self.is_occurrence_safely_encapsulated(command, user_input, occurrence_index) + quote_state = nil # nil = unquoted, "'" = single-quoted, '"' = double-quoted + i = 0 + + while i < occurrence_index + char = command[i] + + if quote_state.nil? + # We're in unquoted context + if char == '\\' + # Backslash escapes the next character in unquoted context + i += 1 + elsif char == "'" + quote_state = "'" + elsif char == '"' + quote_state = '"' + end + elsif quote_state == "'" + # We're in single-quoted context + # In single quotes, nothing is special except the closing single quote + if char == "'" + quote_state = nil + end + elsif quote_state == '"' + # We're in double-quoted context + if char == '\\' + # Skip the next character (it's escaped) + i += 1 + elsif char == '"' + quote_state = nil + end end + + i += 1 + end - # If characters before and after the user input do not match, return false - next false if char_before_user_input != char_after_user_input - - # If user input contains the escape character, return false - next false if user_input.include?(char_before_user_input) - - # Handle dangerous characters inside double quotes - if char_before_user_input == '"' && DANGEROUS_CHARS_INSIDE_DOUBLE_QUOTES.any? { |char| user_input.include?(char) } - next false + # Now check the quote state at the start of user_input + start_quote_state = quote_state + + # Parse through the user_input to see what the quote state would be at the end + user_input.each_char do |char| + if quote_state.nil? + # If we start unquoted, user input is not safely encapsulated + return false + elsif quote_state == "'" + # In single quotes, check if user input contains a single quote + if char == "'" + # User input contains the quote character that would close the encapsulation + return false + end + elsif quote_state == '"' + # In double quotes, check for dangerous characters + if char == '"' + # User input contains the quote character that would close the encapsulation + return false + elsif char == '\\' + # Backslash in double quotes is dangerous + return false + elsif DANGEROUS_CHARS_INSIDE_DOUBLE_QUOTES.any? { |dangerous| char == dangerous } + return false + end end - - next true end - end - def self.get_current_and_next_segments(segments) - segments.each_cons(2).map { |current_segment, next_segment| {current_segment: current_segment, next_segment: next_segment} } + # Verify that after the user_input, we're still in the same quote state + # by checking the character immediately after + end_index = occurrence_index + user_input.length + if end_index < command.length + # Continue parsing to verify the quote is properly closed + char_after = command[end_index] + + if quote_state == "'" && char_after == "'" + # Good: single quote is closed + return true + elsif quote_state == '"' && char_after == '"' + # Good: double quote is closed + return true + else + # The quote is not properly closed immediately after + return false + end + else + # User input is at the end of command, not properly closed + return false + end end # Helper function for sorting commands by length (longer commands first) diff --git a/test/aikido/zen/scanners/shell_injection/helpers_test.rb b/test/aikido/zen/scanners/shell_injection/helpers_test.rb index fdee298f..b43ac566 100644 --- a/test/aikido/zen/scanners/shell_injection/helpers_test.rb +++ b/test/aikido/zen/scanners/shell_injection/helpers_test.rb @@ -153,4 +153,32 @@ def refute_contains_shell_syntax(command, user_input = command) assert_contains_shell_syntax "command -disable-update-check -target https://examplx.com|curl+https://cde-123.abc.domain.com+%23 -json-export /tmp/5891/8526757.json -tags microsoft,windows,exchange,iis,gitlab,oracle,cisco,joomla -stats -stats-interval 3 -retries 3 -no-stdin", "https://examplx.com|curl+https://cde-123.abc.domain.com+%23" end + + test "rejects input between empty quote pairs (CVE fix)" do + # Test case from vulnerability report: printf '';id #'' + # The semicolon is between empty quotes, not inside quotes + refute_is_safely_encapsulated "printf '';id #''", ";id #" + end + + test "rejects input with escaped quotes (CVE fix)" do + # Test case from vulnerability report: echo \";id;echo\" + # The quotes are escaped (literal), not active delimiters + refute_is_safely_encapsulated 'echo \";id;echo\"', ";id;echo" + end + + test "rejects input between adjacent empty quotes" do + refute_is_safely_encapsulated "echo '';rm -rf''", ";rm -rf" + refute_is_safely_encapsulated "printf '';whoami''", ";whoami" + end + + test "rejects input with escaped double quotes around it" do + refute_is_safely_encapsulated 'ls \";cat /etc/passwd;\"', ";cat /etc/passwd;" + refute_is_safely_encapsulated 'echo \";id;\"', ";id;" + end + + test "properly handles backslash escaping in double quotes" do + # In double quotes, backslash escapes the next character + # echo "test\";id;echo\"more" - the quotes after backslashes are literal + refute_is_safely_encapsulated 'echo "test\";id;echo\"more"', ";id;echo" + end end diff --git a/test/aikido/zen/scanners/shell_injection_scanner_test.rb b/test/aikido/zen/scanners/shell_injection_scanner_test.rb index 8ab7c8ee..40711adf 100644 --- a/test/aikido/zen/scanners/shell_injection_scanner_test.rb +++ b/test/aikido/zen/scanners/shell_injection_scanner_test.rb @@ -400,4 +400,28 @@ def stub_payload(source, value, path) assert_equal "ls /app/users/user; rm -rf /app/users", attack.command end end + + test "detects shell injection with empty quote pairs (CVE fix)" do + # Test case from vulnerability report: printf '';id #'' + # The input ;id # is placed between empty quote pairs + assert_attack "printf '';id #''", ";id #" + end + + test "detects shell injection with escaped quotes (CVE fix)" do + # Test case from vulnerability report: echo \";id;echo\" + # The quotes are escaped, so they're literal characters, not delimiters + assert_attack 'echo \";id;echo\"', ";id;echo" + end + + test "detects shell injection when input is between adjacent quotes" do + # Additional test cases for the vulnerability + assert_attack "echo '';rm -rf /;''", ";rm -rf /;" + assert_attack "printf '';whoami;''", ";whoami;" + end + + test "detects shell injection with escaped double quotes" do + # More test cases with escaped quotes + assert_attack 'ls \";cat /etc/passwd;\"', ";cat /etc/passwd;" + assert_attack 'echo \";id;\"', ";id;" + end end