Windows: command injection via the input .ksy file name

[generalmimon]

Recently (at the time of writing in April 2024) there has been a report about improper escaping of command-line arguments on Windows when executing batch files affecting many programming languages: https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/

Most notably, Rust released a patch version 1.77.2 just for the fix of this security issue and published a security advisory describing the problem. For reference, here's the diff between Rust 1.77.1 and 1.77.2, which shows how they fixed it: rust-lang/rust@1.77.1...1.77.2

But it's not specific to Rust. It's more of a general problem on Windows that is not very well known. The problem is that batch files (.bat or .cmd) are executed via cmd.exe, which has different escaping rules than most programs, and this distinction is apparently often overlooked by standard library implementations in many programming languages. As a result, executing batch files via these standard APIs from the particular programming language and passing arguments originating from user input has been found to be often unsafe, because these APIs will fail to escape some special characters properly for cmd.exe and thus there's a good chance that the user can achieve command injection.

I did a quick test to check if KSV is affected (because I knew that kaitai-struct-compiler uses a .bat launcher on Windows). For testing purposes, I added a debug print of ARGV here first, so that it's clear that the improper escaping we'll see really occurs inside KSV, not in the shell:

--- i/bin/ksdump
+++ w/bin/ksdump
@@ -70,6 +70,8 @@ if ARGV.size < 2
   exit 1
 end

+pp ARGV
+
 c = Kaitai::Struct::Visualizer::KSYCompiler.new(options)
 app = Kaitai::Struct::Visualizer::Parser.new(c, ARGV[0], ARGV[1..-1], options)
 exc = app.load

We can also add some more debug prints in ksy_compiler.rb:

1. before the invocation of KSC

   --- i/lib/kaitai/struct/visualizer/ksy_compiler.rb
   +++ w/lib/kaitai/struct/visualizer/ksy_compiler.rb
   @@ -110,6 +111,7 @@ module Kaitai::Struct::Visualizer
          # on Windows.
          args.unshift('--') unless Kaitai::TUI.windows?
   
   +      pp args
          log_str, err_str, status = Open3.capture3('kaitai-struct-compiler', *args)
          unless status.success?
            if status.exitstatus == 127

2. after the invocation of KSC to print the JSON output

   --- i/lib/kaitai/struct/visualizer/ksy_compiler.rb
   +++ w/lib/kaitai/struct/visualizer/ksy_compiler.rb
   @@ -83,6 +83,7 @@ module Kaitai::Struct::Visualizer
        # @return [String] Main class name, or nil if were errors
        def compile_and_load(fns, code_dir)
          log = compile_formats_to_output(fns, code_dir)
   +      pp log
          load_ruby_files(fns, code_dir, log)
        end

---

If you run this in a cmd.exe session (obviously, you need ruby and kaitai-struct-compiler in PATH), the Calculator app will appear, so we indeed have command injection:

C:\temp\kaitai_struct\visualizer>ruby bin\ksdump sample.bin 'hello" & calc.exe .ksy'
["sample.bin", "hello\" & calc.exe .ksy"]
["--ksc-json-output", "--debug", "-t", "ruby", "-d", "C:/Users/pp/AppData/Local/Temp/d20240411-20136-nb8x10", "hello\" & calc.exe .ksy"]
{"hello\""=>{"errors"=>[{"file"=>"hello\"", "path"=>[], "message"=>"hello\" (The filename, directory name, or volume label syntax is incorrect)"}]}}
C:/temp/kaitai_struct/visualizer/lib/kaitai/struct/visualizer/ksy_compiler.rb:151:in `block in load_ruby_files': undefined method `[]' for nil:NilClass (NoMethodError)

        if log_fn['errors']
                 ^^^^^^^^^^
        from C:/temp/kaitai_struct/visualizer/lib/kaitai/struct/visualizer/ksy_compiler.rb:149:in `each'
        from C:/temp/kaitai_struct/visualizer/lib/kaitai/struct/visualizer/ksy_compiler.rb:149:in `each_with_index'
        from C:/temp/kaitai_struct/visualizer/lib/kaitai/struct/visualizer/ksy_compiler.rb:149:in `load_ruby_files'
        from C:/temp/kaitai_struct/visualizer/lib/kaitai/struct/visualizer/ksy_compiler.rb:87:in `compile_and_load'
        from C:/temp/kaitai_struct/visualizer/lib/kaitai/struct/visualizer/ksy_compiler.rb:63:in `block in compile_formats'
        from C:/Ruby32-x64/lib/ruby/gems/3.2.0/gems/tmpdir-0.2.0/lib/tmpdir.rb:99:in `mktmpdir'
        from C:/temp/kaitai_struct/visualizer/lib/kaitai/struct/visualizer/ksy_compiler.rb:63:in `compile_formats'
        from C:/temp/kaitai_struct/visualizer/lib/kaitai/struct/visualizer/ksy_compiler.rb:35:in `compile_formats_if'
        from C:/temp/kaitai_struct/visualizer/lib/kaitai/struct/visualizer/parser.rb:25:in `load'
        from bin/ksdump:77:in `<main>'

Note that KSC clearly only received hello" as the file name - the & calc.exe .ksy part was interpreted by cmd.exe, even though the pp args debug print proves that the Open3.capture3 Ruby function was called with expected arguments.

If we invoke KSC directly, no command injection occurs (so the problem is not in the .bat launcher, for example):

C:\Users\WDAGUtilityAccount>kaitai-struct-compiler.bat --ksc-json-output --debug -t ruby "hello"" & calc.exe .ksy"
{"hello\" & calc.exe .ksy": {"errors": [{"file": "hello\" & calc.exe .ksy","path": [],"message": "hello\" & calc.exe .ksy (The filename, directory name, or volume label syntax is incorrect)"}]}}

After checking the Ruby docs, it seems that Open3.capture3 simply isn't designed to handle any escaping or sanitization at all. Following the links in the documentation starting from Open3.capture3, we get to Kernel#system, which has even a warning about command injection:

1. Open3.capture3:

   The arguments env, cmd and opts are passed to Open3.popen3 except (...)

2. Open3.popen3:

   The parameters env, cmd, and opts are passed to Process.spawn.

3. Process.spawn:

   This method is similar to Kernel#system but it doesn’t wait for the command to finish.

4. Kernel#system:

   This method has potential security vulnerabilities if called with untrusted input; see Command Injection.

So we should either use a different function (I don't know if there is a safe alternative with escaping in the Ruby standard library), or do the escaping/sanitization ourselves.