Recycle bin module#609
Conversation
ledrypotato
requested review from
Marshall-Hallenbeck,
NeffIsBack,
mpgn and
zblurx
as code owners
|
Thanks for the PR! If you still want to work on that module it's best to turn the PR into a "DRAFT", so that we know it is still worked on :) |
|
I have made all the changes that I had planned and also created a PR for the associated documentation for the NetExec wiki. Here is an updated screenshot for the changes that handle displaying the deletion time of files, directories and using a filter to download specific files.
I have gone ahead and removed the draft tag from my PR :) |
|
Any updates? |
|
Hey, oh I didn't even realise there was a previous module for the Recycle Bin by @Dfte 😆 ! |
|
Fyi, until this has been resolved i will turn it into a DRAFT PR so that it is clear it isn't ready for review :) |
|
@ledrypotato have you had a chance to look at the other module and see if you can integrate anything? |
Hey, not yet unfortunately. I should have some more time beginning of next month. |
I removed the registry key logic to fetch the username since it's already given in the metadata file ($I).
ledrypotato
force-pushed
the
RecycleBin-module
branch
from
ad3acd0 to
1a6117a
Compare
|
Hi, I finally had some time to look at this merge. To be honest I moved most of my code that I had into Dfte's module. I removed the registry key logic to fetch the username since we can get that information from the metadata file in the Recycle Bin (files that start with $I).
Here is an updated screenshot of the output when specifying that you want to download files (
Most of the code I originally posted remains the same. I felt it was easier to keep what I already had than try and merge everything. That said I did review the original code to check if things were better in it. Perhaps the Feel free to give any feedback on what should be changed/optimized. |
|
Only took a short look at the code, but looks mostly good from what i can tell 👍 |
|
Yeah the logic remains the same. |
I don't mind either, but it is currently pretty mixed. Not sure what is better but at some point we'll probably enforce one of both, but i'll leave it as is now. |
|
I think underscore is easier to read and I was going to normalize it eventually, maybe after the module arg stuff. |
|
Hey I assume this thread got lost will all the other open PRs. Let me know if anything needs changed before it can be merged. |
More a struggle of good prioritization. But yeah, this PR has waited a long time now, i'll put it up the chain. Thanks for reminding. |
Marshall-Hallenbeck
left a comment
Marshall-Hallenbeck
left a comment
There was a problem hiding this comment.
The download functionality reads the raw bytes and writes them to a file, but there's already functions to download for SMB instead of re-inventing the wheel. Why did you go with this method instead of using an existing one?
| try: | ||
| connection.conn.getFile("C$", file_path, buf.write) | ||
| except Exception as e: | ||
| if "STATUS_FILE_IS_A_DIRECTORY" in str(e): |
There was a problem hiding this comment.
this if else makes no sense, it logs the same thing for both paths
| buf.seek(0) | ||
| return buf.read() | ||
|
|
||
| def convert_filetime_to_datetime(self, filetime): |
There was a problem hiding this comment.
This should probably be a helper function that we can re-use elsewhere.
|
Sorry for the late review. I took a seconds look at the code and realized there are still a few things to do, of which @Marshall-Hallenbeck pointed out a few already. I fixed some minor format issues already.
Yeah actually i think the spidering is much easier to read and also less prone to errors. Recursive execution tends to break because states change or assumptions that have been made break at some point. In addition, please integrate your changes into the existing module without breaking its logic, e.g. as far as I can tell the check on false positives users has been removed, which should remain if there isn't a good reason to remove it. |
NeffIsBack
added
enhancement
|
Hello, thanks for your feedback 😄 I've done quite a few changes to hopefully make the code less prone to error, here are the main ones:
There was already a
I initially wrote this wrapper as I thought it was better to save the small metadata file to memory then parse it without saving it to disk. I have reverted this change and used the existing methods to download files.
The check on the false positives files that can be found in the Recycle Bin is still here. What has changed is the |
|
For reference here is the outdated output: Listing the files in each user's Recycle Bin: 
Downloading a file with a specific name: 
Also as a side note:
|
|
Thanks! I'll look at it soon ™️ If i don't review it in the coming 2-3 weeks please ping me :D |
Whenever you get the chance 😄 |
Of course I forgot😅 Thanks for the ping, I will look at it the next days (hopefully tomorrow). |
NeffIsBack
left a comment
NeffIsBack
left a comment
There was a problem hiding this comment.
Hey, sorry for the delay, tight week. Here is the review tho.
Looking at the structure, we are downloading $I and $R files, mapping metadata information from $I to $R. However, with the current structure this would mean if we retrieve the $R file before we retrieved $I we would miss out on the metadata information.
What about we are just looking for $R and then try to download the respective metadata file? Then we wouldn't have to rely on some internal metadata mapping where everything could exist or not. Thoughts?
| paths = connection.spider("C$", folder="$Recycle.Bin", regex=[r"(.*)"], silent=True) | ||
| filtered_paths = [path for path in paths if not path.endswith(self.false_positive)] | ||
| if not filtered_paths: | ||
| context.log.display("No files found in the Recycle Bin.") |
There was a problem hiding this comment.
I think that should be .info() so it doesn't spam too much when we scan large ranges
| description = "Lists (and downloads) files in the Recycle Bin." | ||
| supported_protocols = ["smb"] | ||
| category = CATEGORY.CREDENTIAL_DUMPING | ||
| false_positive = (".", "..", "desktop.ini", "S-1-5-18") |
There was a problem hiding this comment.
Any reason why Default/Public etc are not included in the list? Do they not have a folder there?
|
|
||
| for sid_directory in connection.conn.listPath("C$", "$Recycle.Bin\\*"): | ||
| paths = connection.spider("C$", folder="$Recycle.Bin", regex=[r"(.*)"], silent=True) | ||
| filtered_paths = [path for path in paths if not path.endswith(self.false_positive)] |
There was a problem hiding this comment.
Maybe we should do the filtering before the spidering, otherwise we waste unnecessary runtime (potentially a lot if these have large dirs)
| # extract the SID as the third component of the path (e.g. C$/$Recycle.Bin/SID/filename) | ||
| sid_dirs = [] | ||
| for path in remote_full_paths: | ||
| sid_dir = path.split("/")[2] | ||
| if sid_dir not in sid_dirs: | ||
| sid_dirs.append(sid_dir) | ||
|
|
||
| # extract the full file path after the SID (e.g. filename or $Rfolder/filename) and remove empty values | ||
| remote_file_paths = ["/".join(path.split("/")[3:]) for path in remote_full_paths if "/".join(path.split("/")[3:])] | ||
|
|
||
| # parse the remote_files_paths and create a mapping of SID to the files that belong to that SID directory | ||
| sid_to_files = {} | ||
| for path in remote_full_paths: | ||
| sid_dir = path.split("/")[2] | ||
| if sid_dir not in sid_to_files: | ||
| sid_to_files[sid_dir] = [] | ||
| sid_to_files[sid_dir].append(path) | ||
|
|
||
| context.log.display(f"Found {len(remote_file_paths)} files in the Recycle Bin across {len(sid_dirs)} user directories. Processing files...") | ||
|
|
||
| # check for each key in sid_to_files, if the value is a list with only 1 element and if that element is the same as the SID directory then we can skip processing that SID directory because it means that there are no files in that SID directory | ||
| for sid_dir, files in sid_to_files.items(): | ||
| if len(files) == 1 and files[0].split("/")[2] == sid_dir: | ||
| context.log.display(f"No files found in C$\\$Recycle.Bin\\{sid_dir}. Skipping...") | ||
| else: | ||
| context.log.highlight(f"Processing directory: C$\\$Recycle.Bin\\{sid_dir} with {len(files) - 1} file(s)...") | ||
| for file in files: | ||
| context.log.debug(f"Found file: {file}") |
There was a problem hiding this comment.
This looks overly complicated, what is that trying to achieve? Can't we just do a log statement in the primary looping logic down below?
Also, it looks to me like currently we are only looping over the last iterated sid_dir as this is set in the loop above and appended to the path in the main for loop down below. I haven't actually tested it yet, but does this indeed iterate over all sids? Maybe I am missing something here tho.
| export_path = join(NXC_PATH, "modules", "recyclebin") | ||
| makedirs(export_path, exist_ok=True) |
There was a problem hiding this comment.
Just do that once at the start of the module execution, otherwise this will be checked in every loop which is unnecessary and in addition is duplicate to the logic in L167-L169
| original_path = match.group(1) | ||
| metadata_map[remote_file_path.replace("$I", "")] = original_path | ||
| size_map[remote_file_path.replace("$I", "")] = file_size_raw | ||
| size_display = f"{file_size_raw // 1024}KB" if file_size_raw >= 1024 else f"{file_size_raw}B" |
There was a problem hiding this comment.
We should probably move the convert_size function from nfs.py to nxc/helpers/misc, add a doc string and use it in nfs and here. See:
Lines 754 to 761 in 1884cd3
| try: | ||
| if sid_directory.get_longname() and sid_directory.get_longname() not in false_positive_users: | ||
|
|
||
| # Extracts the username from the SID | ||
| reg_handle = rrp.hOpenLocalMachine(remote_ops._RemoteOperations__rrp)["phKey"] | ||
| key_handle = rrp.hBaseRegOpenKey(remote_ops._RemoteOperations__rrp, reg_handle, f"SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\ProfileList\\{sid_directory.get_longname()}")["phkResult"] | ||
| username = None | ||
| try: | ||
| _, profileimagepath = rrp.hBaseRegQueryValue(remote_ops._RemoteOperations__rrp, key_handle, "ProfileImagePath\x00") | ||
| # Get username and remove embedded null byte | ||
| username = profileimagepath.split("\\")[-1].rstrip("\x00") | ||
| except rrp.DCERPCSessionError as e: | ||
| context.log.debug(f"Couldn't get username from SID {e} on host {connection.host}") | ||
|
|
||
| # Lists for any file or directory in the recycle bin | ||
| spider_folder = f"$Recycle.Bin\\{sid_directory.get_longname()}\\" | ||
| paths = connection.spider( | ||
| "C$", | ||
| folder=spider_folder, | ||
| regex=[r"(.*)"], | ||
| silent=True | ||
| ) | ||
|
|
||
| false_positive = (".", "..", "desktop.ini") | ||
| filtered_file_paths = [path for path in paths if not path.endswith(false_positive)] | ||
| if filtered_file_paths: | ||
| if username is not None: | ||
| context.log.highlight(f"CONTENT FOUND {sid_directory.get_longname()} ({username})") | ||
| else: | ||
| context.log.highlight(f"CONTENT FOUND {sid_directory.get_longname()}") | ||
|
|
||
| for path in filtered_file_paths: | ||
| # Returned path look like: | ||
| # $Recycle.Bin\S-1-5-21-4140170355-2927207985-2497279808-500\/$I87021Q.txt | ||
| # Or | ||
| # $Recycle.Bin\S-1-5-21-4140170355-2927207985-2497279808-500\/$R87021Q.txt | ||
| # $I files are metadata while $R are actual files so we split the path from the SID | ||
| # And check that the filename contains $R only to prevent downloading useless stuff | ||
|
|
||
| if "$R" in path.split(sid_directory.get_longname())[1] and not path.endswith(false_positive): | ||
| # Create the export path | ||
| export_path = join(NXC_PATH, "modules", "recyclebin") | ||
| makedirs(export_path, exist_ok=True) | ||
|
|
||
| # Formatting the destination filename | ||
| file_path = path.split("$")[-1].replace("/", "_") | ||
| filename = f"{connection.host}_{username if username else sid_directory.get_longname()}_recyclebin_{file_path}" | ||
| dest_path = abspath(join(export_path, filename)) | ||
| try: | ||
| with open(dest_path, "wb+") as file: | ||
| connection.conn.getFile("C$", path, file.write) | ||
| except Exception as e: | ||
| if "STATUS_FILE_IS_A_DIRECTORY" in str(e): | ||
| context.log.debug(f"Couldn't open {dest_path} because of {e}") | ||
| else: | ||
| context.log.fail(f"Failed to write recyclebin file to {filename}: {e}") | ||
| else: | ||
| context.log.highlight(f"\t{dest_path}") | ||
| found += 1 | ||
| except DCERPCSessionError as e: | ||
| if "ERROR_FILE_NOT_FOUND" in str(e): | ||
| continue | ||
| else: | ||
| context.log.fail(f"Error opening {sid_directory.get_longname()} on host {connection.host} because of {e}") | ||
| remove(dest_path) | ||
| context.log.debug(f"Deleted metadata file: {dest_path}") | ||
| except Exception: | ||
| context.log.debug(f"Could not delete metadata file: {dest_path}") |
There was a problem hiding this comment.
If we remove the file anyway we should probably use BytesIO, so we can skip all that creating dir, writing to disk etc. logic. See:
NetExec/nxc/modules/gpp_password.py
Lines 46 to 47 in 1884cd3
There was a problem hiding this comment.
God, that diff looks crazy due to the "removal" detected by git. This belongs only to the 4 addition lines
| except Exception as e: | ||
| context.log.debug(f"Could not get file size for {remote_file_path} via listPath: {e}") | ||
|
|
||
| size_display = f"{file_size_raw // 1024}KB" if file_size_raw and file_size_raw >= 1024 else f"{file_size_raw}B" if file_size_raw else "unknown" |
| with open(path, "wb") as f: | ||
| connection.conn.getFile("C$", remote_full_path, f.write) | ||
| context.log.success(f"Recycle Bin file {remote_file_path} written to: {path}") | ||
| f.close() |
| if (len(remote_file_path) <= 8): # if it's a folder we can't download it and we should skip it | ||
| context.log.debug(f"Skipping download of {remote_file_path} because it appears to be an empty directory.") |
There was a problem hiding this comment.
Why is it a folder when the len is <= 8? Isn't that a check in the name of the dir/file?
|
Thanks for the in-depth review. I won't have time to check the review this week and I won't be here for the next few weeks so it will have to wait a bit 😄 . |
No worries, we are not in a rush :D |
3 participants
Description
This PR adds the Recycle Bin module that will list files in the Recycle Bin. It will parse the associated metadata files in the Recycle Bin to display the "Original Location" of the deleted file, this can give a good indication whether or not the file is interesting or not. You can also download files using the module options.
I have a few things on my TODO list that I will implement as soon as I can as well as update the NetExec documentation.
Feel free to make any comments on the implementation/development as it's not my speciality 😆!
Type of change
How Has This Been Tested?
I have tested this module against my local Windows 11 VM (Microsoft Windows 11 Enterprise Evaluation) from my Kali VM running Python 3.12.8.
Screenshots:
Here is an example of deleted files in the Recycle Bin:
We can list files in the Recycle Bin with this command:
or download them with this command:
Checklist:
poetry run python -m ruff check . --preview, use--fixto automatically fix what it can)TODO: