Skip to content

Strip periods, spaces for batch file filtering on Windows#15573

Merged
straight-shoota merged 1 commit intocrystal-lang:masterfrom
GeopJr:fix/process/win32-batcmd-protection-bypass
Mar 27, 2025
Merged

Strip periods, spaces for batch file filtering on Windows#15573
straight-shoota merged 1 commit intocrystal-lang:masterfrom
GeopJr:fix/process/win32-batcmd-protection-bypass

Conversation

@GeopJr
Copy link
Contributor

@GeopJr GeopJr commented Mar 20, 2025

fix: #15560

As discussed in the linked issue, the options on the table are either GetFullPathNameW or stripping trailing . and . This PR does the latter.

Here's a benchmark between them, the difference seems insignificant, considering executing is going to take longer. Stripping is faster anyway.

require "benchmark"

File.write("foo.bat", "calc.exe") unless File.exists?("foo.bat")
COMMAND = "foo.bat . .  .. #{ARGV[0]? || ""}"

Benchmark.ips do |x|
  x.report("GetFullPathNameW") do
    wstr_path = Crystal::System.to_wstr(COMMAND)
    Crystal::System.retry_wstr_buffer do |buffer, small_buf|
      len = LibC.GetFullPathNameW(wstr_path, buffer.size, buffer, nil)
      if 0 < len < buffer.size
        break String.from_utf16(buffer[0, len])
      elsif small_buf && len > 0
        next len
      else
        raise ::File::Error.from_winerror("Error resolving real path", file: COMMAND)
      end
    end
  end

  x.report("rstrip") do
    COMMAND.rstrip(". ")
  end
end

# GetFullPathNameW 967.44k (  1.03µs) (± 2.75%)   112B/op   2.75× slower
#           rstrip   2.66M (376.29ns) (± 1.42%)  32.0B/op        fastest

Draft until the CI finishes because I haven't actually tested on Windows.

I don't like the commit name that much but it's the most descriptive one I could come up with.

@GeopJr GeopJr marked this pull request as ready for review March 20, 2025 14:20
@Blacksmoke16 Blacksmoke16 added kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:windows Windows support based on the MSVC toolchain / Win32 API security topic:stdlib:system labels Mar 20, 2025
@GeopJr GeopJr force-pushed the fix/process/win32-batcmd-protection-bypass branch from ef9aa44 to a6ef786 Compare March 20, 2025 15:55
@ysbaddaden ysbaddaden added this to the 1.16.0 milestone Mar 25, 2025
@straight-shoota straight-shoota changed the title Strip periods and spaces during batch file filtering on Windows Strip periods, spaces for batch file filtering on Windows Mar 27, 2025
@straight-shoota straight-shoota merged commit eae2726 into crystal-lang:master Mar 27, 2025
36 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:windows Windows support based on the MSVC toolchain / Win32 API security topic:stdlib:system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Batch implicit execution protection bypass on Windows

4 participants