Skip to content

Fix mkdir post API to behave consistently across session types#20944

Open
Nayeraneru wants to merge 2 commits intorapid7:masterfrom
Nayeraneru:fix-mkdir-consistent
Open

Fix mkdir post API to behave consistently across session types#20944
Nayeraneru wants to merge 2 commits intorapid7:masterfrom
Nayeraneru:fix-mkdir-consistent

Conversation

@Nayeraneru
Copy link

@Nayeraneru Nayeraneru commented Feb 10, 2026

Summary

This PR fixes #20697
Currently, powershell, Windows shell, and Unix (mkdir -p) automatically create intermediate directories. However, Windows Meterpreter relies on CreateDirectoryW, which fails if parent directories do not exist. This leads to inconsistent behavior depending on the session type.

Change

This PR introduces a recursive helper method meterpreter_mkdir_p and meterpreter_parent_directory to emulate mkdir -p behavior for Meterpreter sessions.

The implementation:

  • Checks whether the target directory already exists.
  • Attempts to create the directory.
  • If creation fails due to a missing parent, it recursively creates the parent directory first.
  • Retries directory creation after ensuring the parent exists.
  • Normalizes path separators for cross-platform compatibility.
  • Handles Windows drive roots correctly (e.g., C:\).

@jvoisin
Copy link
Contributor

jvoisin commented Feb 10, 2026

This looks a bit overengineered compared to a simple for loop splitting on path separator and creating folders. Or even better, adding an mkdir_p method to meterpreter tbh.

@Nayeraneru
Copy link
Author

This looks a bit overengineered compared to a simple for loop splitting on path separator and creating folders. Or even better, adding an mkdir_p method to meterpreter tbh.

Thanks for the feedback. I agree that the iterative approach is simpler and more straightforward in many cases.

For paths like C:\Temp\A\B\C (where C:\Temp exists but A is missing), the loop version might fail if it doesn’t handle C: and separators correctly, or it would require manual parent checking. The recursive version automatically creates all missing directories (A, B, C).

For Unix-style paths like /tmp/A/B/C, the loop version works only if the parent directories already exist; otherwise, it fails unless extra checks are added. The recursive version works regardless of which intermediate directories exist.

So while iteration is simpler and easier to read, recursion mirrors mkdir -p behavior and handles missing parents naturally, providing safer and more consistent behavior in all scenarios.

@msutovsky-r7 msutovsky-r7 moved this from Todo to Waiting on Review in Metasploit Kanban Feb 10, 2026
@msutovsky-r7 msutovsky-r7 self-assigned this Feb 10, 2026
@jvoisin
Copy link
Contributor

jvoisin commented Feb 10, 2026

What's wrong with something like his:

directories = directory.s.split(s.fs.file.separator)
if session.platform == 'windows'
  current_dir = directories.pop()  # to get the drive letter
end

current_dir = ''
directories do | dir |
  current_dir += s.fs.file.separator + dir
  session.fs.dir.mkdir(current_dir) unless directory?(current_dir)
end

@Nayeraneru
Copy link
Author

What's wrong with something like his:

directories = directory.s.split(s.fs.file.separator)
if session.platform == 'windows'
  current_dir = directories.pop()  # to get the drive letter
end

current_dir = ''
directories do | dir |
  current_dir += s.fs.file.separator + dir
  session.fs.dir.mkdir(current_dir) unless directory?(current_dir)
end

Thanks for the suggestion and for sharing the example, It is very helpful.

I’ve updated the code to use the same iterative approach you described. One small change I made was around Windows drive handling: instead of deriving the drive from the split path, I detect the drive root (C: / C:\) directly from the normalized path, which avoids some ambiguity depending on how the path is structured.
I also normalize path separators up front and build the path incrementally to avoid duplicate separators, while keeping the same mkdir -p behavior overall.

Does this look reasonable to you, or would you suggest any further tweaks?

def meterpreter_mkdir_p(path)
  return nil if directory?(path)

  separator = session.fs.file.separator
  normalized = path.tr('\\/', separator)
  directories = normalized.split(separator)
  current_path = ''

  if normalized.match?(/\A[a-zA-Z]:#{Regexp.escape(separator)}?/)
    current_path = "#{directories.shift}#{separator}"
  elsif normalized.start_with?(separator)
    current_path = separator
  end

  directories.each do |dir|
    next if dir.empty?

    current_path =
      if current_path.end_with?(separator) || current_path.empty?
        "#{current_path}#{dir}"
      else
        "#{current_path}#{separator}#{dir}"
      end

    session.fs.dir.mkdir(current_path) unless directory?(current_path)
  end

  nil
end

note: meterpreter_parent_directory function is removed from the code

@msutovsky-r7 msutovsky-r7 moved this from Waiting on Review to In Progress in Metasploit Kanban Feb 11, 2026
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue should be probably addressed on Meterpreter side, to avoid overhead of sending data for each path level. The specs should be kept though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

#mkdir is inconsistent

3 participants