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

Facedancer --suggest mode doesn't print suggestions, as Ctrl-C is no longer caught #118

Closed
miek opened this issue Sep 19, 2024 · 7 comments · Fixed by #119
Closed

Facedancer --suggest mode doesn't print suggestions, as Ctrl-C is no longer caught #118

miek opened this issue Sep 19, 2024 · 7 comments · Fixed by #119
Assignees
Labels

Comments

@miek
Copy link
Member

miek commented Sep 19, 2024

When running facedancer examples with the --suggest argument, pressing Ctrl-C to end the script just prints a backtrace instead of printing the aggregated suggestions. It used to catch the exception, but that got removed in #92

@mossmann
Copy link
Member

@kauwua Do you have any objection to reverting ea8c2a6?

@kauwua
Copy link
Contributor

kauwua commented Sep 25, 2024

I don't think this commit should be reverted : if Facedancer catches the exception without rethrowing it, it prevents Facedancer users from catching Ctrl-C. I personnaly needed to catch Ctrl-C in a script that was creating devices in a loop and Facedancer was catching the KeyboardInterrupt without rethrowing it so I had no way to get a KeyboardInterrupt event (to print my own results before the script ends). If I remember correctly it also interfered with the way Ctrl-C is used by the Python interpreter to stop the script.

The emulation of the device (https://github.com/greatscottgadgets/facedancer/blob/cfb3d5899463b58b097ed6860099dd0320df8a20/facedancer/devices/__init__.py#L40C1-L44C43) could be wrapped like this to ensure suggestions will always be printed (finally will always be executed : it will run after any except handler and before any uncaught exception)

try:
    # Run the relevant code, along with any added coroutines.
    device.emulate(*coroutines)
finally:
    if args.suggest:
        device.print_suggested_additions()

@antoinevg
Copy link
Member

antoinevg commented Sep 26, 2024

How about: #119

@kauwua
Copy link
Contributor

kauwua commented Sep 26, 2024

catching KeyboardInterrupt would be ok in default_main since it's a convenience function (the main issue was Facedancer catching Ctrl-C directly in the core library and not exiting). this way you could print the suggestions, exit cleanly (with exit probably) to remove the ugly backtrace and users could create their own main if they want.

#119 PR works for me as well

@miek
Copy link
Member Author

miek commented Sep 27, 2024

I'm reluctant to introduce a new keyboard shortcut if we don't need to, I like the suggestion of catching KeyboardInterrupt in default_main instead.

@mossmann
Copy link
Member

@antoinevg plans to rework #119 so that the interrupt is caught in default_main and the ctrl-e solution is moved to an example.

@antoinevg
Copy link
Member

PR updated, feedback appreciated plz! #119

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

Successfully merging a pull request may close this issue.

4 participants