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 ServerName argument to srvs.hNetrShareEnum #947

Closed
wants to merge 1 commit into from
Closed

Add ServerName argument to srvs.hNetrShareEnum #947

wants to merge 1 commit into from

Conversation

cnotin
Copy link
Contributor

@cnotin cnotin commented Sep 7, 2020

I have an SMB server for which impacket smbconnection.listShares() doesn't return as many shares as Windows explorer shows me.
Impacket only returns "C$", "F$" and a few others, and "IPC$", "print$" and "prnproc$", while Windows sees many more shares which are not only technical/admin shares but also business ones.

Through a packet capture analysis I found the difference being that Windows fills the "ServerName" field in the NetShareEnumAll request which impacket doesn't (or actually fills it with a NULL byte only coming from b4cacea):
https://github.com/SecureAuthCorp/impacket/blob/d84fca225175729bdf215adca10f3b3bd5a84733/impacket/dcerpc/v5/srvs.py#L3095-L3097

We can observe that Windows fills it with "\\<ip><NULL>"
Even though the specification mentions that this field can be NULL and that the server MUST remove any preceding "\\"

This code fixes it for smbconnection.listShares() only (which is what I need at the moment...). Would you like me to add something similar to all other structures in srvs.py that have a "ServerName" field?

I tested it on the problematic server and a few others, but I can run a larger scale test :)
EDIT: I did a larger scale test and I did not notice any regression

@mohemiv
Copy link
Contributor

mohemiv commented Sep 7, 2020

Hello @cnotin!

I've just read the specification by your link, and I've found this:
If the Level member is 503, the server MUST return all shares in SHARE_INFO_503_I structures. Otherwise, the server MUST return the shares in which share.shi503_servername matches ServerName.

Isn't this the right way to list all the shares?

@cnotin
Copy link
Contributor Author

cnotin commented Sep 7, 2020

Thanks @mohemiv! That's a very good idea I'll try that!

@cnotin
Copy link
Contributor Author

cnotin commented Sep 8, 2020

I did a quick test and it wasn't very successful (the server answered with an ACCESS_DENIED error) but it didn't dig too much!

@anadrianmanrique
Copy link
Contributor

Hi, I merged these changes in the context of #1721, as this PR needs to be rebased.
Thank you!

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.

3 participants