Skip to content

Conversation

0xGrooted
Copy link

Hey there,
My code replaces the deprecated, manual file handling logic within cmd_edit with the robust, high-level client.fs.file.edit(path) utility.

The manual logic was flawed because the remote file was removed immediately when the handler attempted to open it to read its contents, causing complete data loss if the local editor crashed or the session was interrupted before re-upload.

The fix ensures the edit command now follows the atomic procedure:

  • Safely downloads the remote file to a local temporary file.
  • Launches the local editor ($EDITOR).
  • Uploads the file back to the remote host (overwriting the original) only upon successful exit from the editor.

This (hopefully) Fixes #20574.

# Try to download the file, but don't worry if it doesn't exist
client.fs.file.download_file(temp_path, client_path) rescue nil
if args.empty? || args.length > 1
print_error("Usage: edit <file>")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can use cmd_edit_help here as well.

if (system("#{editor} #{temp_path}") == true)
client.fs.file.upload_file(client_path, temp_path)
begin
client.fs.file.edit(path)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this part is working, see:

[*] Started reverse TCP handler on 192.168.3.7:4242 
[*] Sending stage (203846 bytes) to 10.5.132.145
[*] Meterpreter session 1 opened (192.168.3.7:4242 -> 10.5.132.145:50452) at 2025-10-02 14:29:12 +0200


meterpreter > 
meterpreter > sessions 
Usage: sessions [options] or sessions [id]

Interact with a different session ID.

OPTIONS:

    -h, --help           Show this message
    -i, --interact <id>  Interact with a provided session ID

meterpreter > 
meterpreter > edit desktop.ini 
[-] An error occurred during edit: NoMethodError undefined method `edit' for #<Class:0x0000744abf2bae70>

Also, in the original code, there was a piece of code responsible for expanding the path - would you mind explaining why you removed that part?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you so much for the review and for catching that NoMethodError. You are right, Apologies :). I completely misunderstood the API and mistakenly assumed a high-level client.fs.file.edit method existed when it does not. I should have verified that...

To answer your question about the path expansion, I removed it under the same incorrect assumption that a built-in edit utility would handle that internally. I will add that back in ASAP.

So i need to do the following:
Re-implements the logic correctly using the existing download_file and upload_file methods to create an atomic operation, which was my original intent.
Re-add the path expansion code.
Uses cmd_edit_help for the usage message as you suggested in your first comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

meterpreter edit truncates file before editing (data loss) — Metasploit 6.4.91-dev (apt)
2 participants