Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop using cmd.exe to keep current directory #2488

Merged
merged 4 commits into from
Oct 24, 2024

Conversation

robbert1978
Copy link
Contributor

Because cmd.exe can't parse the file path of WSL, it automatically drop the current directory to /mnt/c/Windows.

Screenshot 2024-10-13 173943

We can call wt.exe directly to keep the current directory. Because some CTF challenges needs patching run path of the binary to ., so keeping exactly the current directory is convenient for debugging.

Screenshot 2024-10-13 174014

@@ -367,13 +367,13 @@ def run_in_new_terminal(command, terminal=None, args=None, kill_at_exit=True, pr
with open('/proc/sys/kernel/osrelease', 'rb') as f:
is_wsl = b'icrosoft' in f.read()
if is_wsl and which('cmd.exe') and which('wsl.exe') and which('bash.exe'):
terminal = 'cmd.exe'
args = ['/c', 'start']
terminal = 'wt.exe'
Copy link
Member

Choose a reason for hiding this comment

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

We cannot use wt.exe if the user does not have it installed. Also, the problem is not cmd.exe but start (which probably can take an extra cwd argument? not sure, I personally do not have any Windows machine thankfully, but you can run start /? in cmd to get some idea of whether it is possible). Otherwise, we could run wt.exe without start and the native terminal with start.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe we can use powershell (yes , it can prase the file path of WSL ), but it will cost much more resources than cmd.

Copy link
Contributor Author

@robbert1978 robbert1978 Oct 13, 2024

Choose a reason for hiding this comment

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

Ah, I think I have a way to deal with cmd:

        else:
            is_wsl = False
            if os.path.exists('/proc/sys/kernel/osrelease'):
                with open('/proc/sys/kernel/osrelease', 'rb') as f:
                    is_wsl = b'icrosoft' in f.read()
            if is_wsl and which('cmd.exe') and which('wsl.exe') and which('bash.exe'):
                terminal    = 'cmd.exe'
                args        = ['/c', 'start']
                distro_name = os.getenv('WSL_DISTRO_NAME')
                current_dir = os.getcwd()

                # Split pane in Windows Terminal
                if 'WT_SESSION' in os.environ and which('wt.exe'):
                    args.extend(['wt.exe', '-w', '0', 'split-pane'])
                if distro_name:
                    args.extend(['wsl.exe', '-d', distro_name, '--cd', current_dir, 'bash', '-c'])
                else:
                    args.extend(['bash.exe', '-c'])

image

image

@peace-maker
Copy link
Member

A similar patch is included in #2470. Did I miss something? The --cd argument looks nicer than that cd cwd; <cmd> workaround though.

Copy link
Member

@peace-maker peace-maker left a comment

Choose a reason for hiding this comment

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

Maybe we can switch to launch wsl.exe without -d if the distro_name is unknown instead of bash.exe and use --cd there too. But the other change of cd cwd;realcommand is still here too.

Thanks for the contribution and we can clean this up further later.

@peace-maker peace-maker merged commit 7dceedd into Gallopsled:dev Oct 24, 2024
13 of 14 checks passed
peace-maker pushed a commit to peace-maker/pwntools that referenced this pull request Oct 26, 2024
* Stop using cmd.exe to keep current directory

* Update misc.py

* Update misc.py, deal with cmd.exe

* Update misc.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants