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

Multiple Enhancements and Bugfixes - Feature: Bypass STATUS_SHARING_VIOLATION errors #30

Open
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

covertivy
Copy link

@covertivy covertivy commented Jan 17, 2025

PR Summary

As this PR is not being accepted any time soon I thought I might add some value to it... 😄

Initially I Redesigned & Re-Implemented all of the Triage Classes in order to create a better codebase as well as fix a bug in which the false_positive variable could not be changed by the user at all - as it was hardcoded.

Later, following my recent impacket PR, I decided to add support for bypassing STATUS_SHARING_VIOLATION with no hassle at all.

I will describe both features in depth below.
enjoy!

Feature: Bypass STATUS_SHARING_VIOLATION errors with ease

This is the original PR I created for impacket.
What I discovered is that we can actually read files that are open by other processes with no restriction at all (as long as they are not system files) as a feature.

The problem stems from the first implementation of impacket's SMB and SMB3 classes that restrict (by default) sharing access rights to other processes.
This means that impacket is forcing other processes to have READONLY handles to files, if a process with write permissions has an open handle - impacket fails and raises an STATUS_SHARING_VIOLATION (or STATUS_ACCESS_DENIED).

Why is this important?
Basically what everyone else did to solve this issue is overcomplicated as fuck and really bad...
This means that all (extremely awful) solutions such as killing the browser of the user etc... can be COMPLETELY AVOIDED.

I hope this feature gets this PR merged as soon as possible so dumping files and credentials remotely is nice and easy!

Refactor: Redesign & Re-Implement Triage Classes

There are a couple of noticeable problems with the triage classes:

  • They should be implemented with OOP (Object Oriented Programming) in mind, but they are not.
  • For the Triage classes that implement the false_positive property, it is hard coded and not changeable.
    • This is especially problematic when we use dploot in DonPAPI - where the false_positive parameter does almost nothing (as everything is hardcoded).

I sought to fix both problems, this PR does not change any of the logic of the project so there is nothing to worry about in that sense.
This PR will help me create a following PR on DonPAPI that implements better filtering of user folders and false positives.

This is related to this issue I created for the DPAPI project.

Thanks! I am free to answer questions as always!

This makes more sense as this class is used by all Triage classes in the project.
For some reason we define the 'false_positive' property for (almost) every Triage class.
This will help us reduce redefinitions and messy code.
This make much more sense as the entire project is aimed for modularity.
Now we can utilize the power of Object Oriented Programming ;)
This is related to my fix in the impacket source code.
@covertivy covertivy changed the title Refactor: Redesign & Re-Implement Triage Classes Multiple Enhancements and Bugfixes - Feature: Bypass STATUS_SHARING_VIOLATION errors Feb 9, 2025
@covertivy
Copy link
Author

You guys should merge this PR ASAP as it will greatly enhance the usage and reliability of both DPLoot and DonPAPI!

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.

1 participant