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

Stat files directly to avoid issues with Windows _wstat #195

Closed
wants to merge 1 commit into from

Conversation

umerov1999
Copy link
Contributor

Fix FS Stat on Windows XP

Fix FS Stat on Windows XP
@metasploit-public-bot
Copy link

Can one of the admins verify this patch? For more information see: https://github.com/rapid7/metasploit-payloads/wiki/CI-Testing

@OJ
Copy link
Contributor

OJ commented Apr 26, 2017

Thanks @umerov1999, may I please suggest that you read the documentation on contributing for future PRs, so that the required information is included on submission?

Would you please update your PR to include some detail such as:

  • Reason for the change
  • Sample run through of your code in action
  • Details on exactly which environments this intends to impact

I appreciate the submission. Thank you!

@OJ
Copy link
Contributor

OJ commented Jun 13, 2017

Bump @umerov1999, would you mind making the adjustments please? Can you please provide failure scenarios as well thanks?

@umerov1999
Copy link
Contributor Author

@OJ, On Windows XP if I use 'ls' or 'download' - I have message: This function doesn't support in this system. My Commit fix this

@wwebb-r7
Copy link
Contributor

Hi @umerov1999 , could you provide the build number and all other relevant information from the system you are testing on? I am not able to reproduce the problem on any XP machine that I have tried.

@busterb
Copy link
Member

busterb commented Jul 11, 2017

Nearest I can tell, _wstat is available as of Windows NT. I tried running Meterpreter on Windows 98 but couldn't get the stager to even run.

@busterb
Copy link
Member

busterb commented Jul 21, 2017

This looks to be derived from the Python implementation win32_wstat:

python/cpython@1469466#diff-a6f29e907cbb5fffd44d453bcd7b77d5R741

+- Use Win32 API to implement os.stat/fstat. As a result, subsecond timestamps
+  are reported, the limit on path name lengths is removed, and stat reports
+  WindowsError now (instead of OSError).

The changelog there provides a bit more historical reasoning behind this (and we pretty much already have this function in the python extension source). There is also an issue at least in Windows Vista where _wstat does not work with symbolic links, and it also has a 4GB file size limit allegedly: https://manojhirway.blogspot.com/2013/06/wstat-on-file-larger-than-4gb.html

Seems like a reasonable change with this context, but given the code is so similar, we should add license / attribution.

@busterb
Copy link
Member

busterb commented Jul 21, 2017

Interestingly, Python 3 dropped this and went back to _wstat

@busterb
Copy link
Member

busterb commented Jul 22, 2017

After some more look at Python 3's history, it may be that dropping this was accidental since there were a few reworkings of file IO over time. I have some changes that I'm testing now on top of this, but think this is OK with some tweaks and proper docs. Thanks!

@busterb busterb changed the title Update fs_win.c Stat files directly to avoid issues with Windows _wstat Jul 22, 2017
@busterb busterb self-assigned this Jul 22, 2017
@busterb
Copy link
Member

busterb commented Jul 22, 2017

I rebased this on master, additional commits:

94f4147 - give attribution (2 minutes ago)
9118645 - simplify and reduce logic (5 minutes ago)

Keeping the original name of win32_wstat was fine, since this is a static function (no symbols exported).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants