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

Add syscalls #101

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

Add syscalls #101

wants to merge 8 commits into from

Conversation

S4ntiagoP
Copy link
Contributor

I added support for direct system calls using syswhispers2.

Due to how the loader is coded, I had to make some changes to SW2 so that it doesn't use global variables.
What I did is pass the syscall table as the last parameter for each syscall.
I works for both x64 and x86 (not for WoW64)

I also removed some APIs that are no longer used (as they have been replaced by syscalls).

Note:
This PR is built on top of this one: #100

Hope you like it!

@TheWover TheWover self-requested a review January 12, 2022 14:02
@TheWover TheWover self-assigned this Jan 12, 2022
@TheWover TheWover added the enhancement New feature or request label Jan 12, 2022
@TheWover TheWover added this to the v0.9.3 milestone Jan 12, 2022
This should fix a bug when loading binaries made with msfvenom
@TheWover
Copy link
Owner

This also fixes errors with the Python module in Python 3.8

@S4ntiagoP
Copy link
Contributor Author

That's because it sits on top of a previous PR that fixes that, maybe I should have created just one large PR instead of several smaller ones 😛

@TheWover
Copy link
Owner

TheWover commented Dec 5, 2022

I noticed that there are a couple PRs on the source repo/branch about stricmp.

This is a note to myself to adjust the code accordingly after merging.

@TheWover
Copy link
Owner

TheWover commented Dec 6, 2022

So I have considered how to integrate this and made the following decision:

  1. I love this addition and appreciate the work to make it happen.

  2. I'm not going to integrate it into the master branch. While it does make clear OpSec improvements, it reduces the reliability and flexibility of the project because syscalls occasionally change their function signature / parameters and the lack of WOW64 support. I see donut primarily as a proof-of-concept project that demonstrates TTPs, but does not go to all possible lengths to ensure operational security. That is left as an exercise to the user or is extra cost that may be incurred by full-time operators / organizations who choose to incorporate the tool into their operations or products. As such, I prioritize reliability and flexibility over an absolute degree of OpSec. Additionally, there would be an added maintenance burden on the project for adjusting the syscall usage if Microsoft changes those syscalls in the future.

  3. However, I do want to provide it to users as a clear, documented option.

  4. So, once v1.0 is released into master, I will create a branch that forks from that release and includes this PR. I will also update the readme in master to instruct people who prefer syscalls that they can find a version that fits that requirement in the syscalls branch. I will also add a disclaimer that it will not be supported or updated as regularly so use of it assumes a maintenance burden and the reliability/flexibility tradeoffs.

Meanwhile I will leave up the PR. Thanks again for the work and the other two PRs that this was built upon!

@S4ntiagoP
Copy link
Contributor Author

You make a great point, thank you for the detailed explanation, I completely agree 😄

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

Successfully merging this pull request may close these issues.

2 participants