Updated smb_enumshares to support share spidering #3569
Conversation
| @@ -44,7 +45,11 @@ def initialize(info={}) | |||
|
|
|||
| register_options( | |||
| [ | |||
| OptBool.new('DIR_SHARE', [true, 'Show all the folders and files', false ]), | |||
| OptBool.new('SpiderShares', [false, 'Spider shares recursively', false]), | |||
| OptBool.new('VERBOSE', [true, 'Show detailed information when spidering', true]), | |||
There was a problem hiding this comment.
You should remove this. This option already exists globally, I believe.
There was a problem hiding this comment.
It does, but I only added it so that users can see it there, as opposed to only being available via "Show Advanced". Is this going to be ok?
|
Still working on a few changes. Thanks so much thus far guys. |
|
Modified a few things based on the suggestions; however, I'm not quite sure about the VERBOSE option up there. I thought it'd be good to have it shown considering it's pretty important if spidering is enabled. |
|
I remember working on this module way back. I'll handle the review and testing again.... will take a while because it's a lot of code, and it's SMB (and nobody likes SMB modules being broken).
Your concern is valid. But you are also changing the default verbose to true, which can generate a lot of output against a large network. Is this what you actually want? There's a reason we set the default to false. Also, the module does need to take advantage of the |
|
Hey wchen-r7, Thanks for the suggestions. I've checked again and I believe I didn't use vprint_status in two locations because I wanted the print statements to go through only if the VERBOSE option was disabled. They're supposed to print out information only when verbose is disabled so that the user can keep track of what system the module's on. Is this ok? |
|
That works for me. Pulling now and testing. |
|
Sounds great. Thanks man! |
|
Ok, I just finished testing. You did pretty good, I only found one bug. I've already fixed it and will go ahead and land it to master. However, for documentation purposes, I'll leave my review notes here: Bug: nil return value in eval_host() So I set up some Windows boxes: XP, 2003, 7, and 8. While on 8, I set
This was actually an exception caught by the 345. while subdirs.length > 0
'NoMethodError' 'undefined method `each' for nil:NilClass'Due to the following line: 292. files.each do |f|So the question is: Why is Solution If Other changes in the fix I also had to make some other changes in order to pass msftidy.
And then I retested everything on the same boxes before committing. |
|
Gotcha. I noticed the datastore options being set too from a previous author, but I didn't modify it, assuming it'd still be fine. Thanks so much for your advice regarding the few issues. Good information to know for future contributions. 👍 |
Yeah, we used to do that a lot, and then we learned about the race condition, so we stopped doing it. Definitely not your mistake. You just gave me an excuse to finally get off my lazy ass to fix it :-) |
|
Ah, lol! :P |
My first PR involved me submitting an smb_spider module that enumerated shares from systems accessible via local administrator credentials. From this list, it would be possible to grep the results and look for sensitive files (i.e., password files, files containing PII, etc.).
I've taken the suggestion and modified smb_enumshares to also be able to provide this functionality. I've removed the "DIR_SHARE" option that was there before, since this only provided the directory listing of the root shares accessible, and added some spidering options instead.
Quick description of four options I've added:
Screenshots
Smb_enumshares options:
Spidering the root of shares (spidering only user profiles in C$ turned on):
Spidering the root of all shares (spidering only user profiles in C$ turned off):
Console Output
Console Output (verbosity disabled)
Let me know if there are any questions or suggestions.
Thanks.