ModOrganizer2 / modorganizer

Mod manager for various PC games. Discord Server: https://discord.gg/ewUVAqyrQX if you would like to be more involved
http://www.nexusmods.com/skyrimspecialedition/mods/6194
GNU General Public License v3.0
2.2k stars 163 forks source link

Directories starting with a dot drive MO2 crazy (and MO2 may "crash" process on long paths) #134

Closed erasmux closed 6 years ago

erasmux commented 6 years ago

When investigating a LOOT 0.12 crash with the latest developmental build (MO 2.1 build 2/12/2017), I stumbled upon two bugs. I will start with a test scenario which demonstrates the more severe bug IMHO. After that I will detail why MO2 + long paths are causing ungraceful termination of the running process.

A test case which reproduces the problem for me:

  1. Add the following file to one of the mods: readmes\.bla\hello.txt
  2. Check file structure under Data\readmes with MO2 (I did this by running FreeCommander). The problem is that Data\readmes\.bla points back to Data\readmes, so you can reach paths like: Data\readmes\.bla\.bla\.bla\.bla\.bla\.bla\.bla\.bla\.bla\.bla\.bla\.bla\.bla\.bla and if a program will try to recurse the directory structure it will likely cause a crash (more on this later). Another side of effect of this is that hello.txt appears under Data\readmes. I think MO2 is interpreting .bla as ..

Oddly enough, depending on the directory structure, I don't always see the .bla directory. For example if I put .bla directly under Data or use docs instead of readmes in the above example I don't see the .bla folder at all. The hello.txt will still appear in the parent folder.

I stumbled upon this bug because I managed to get a .vs folder added to the FNIS tools folder (when I let VS debug unrelated crashes MO2 is causing in FNIS...). This caused LOOT 0.12 with MO2 to terminate in the following manner:

...
07 0812baf4 630b93f3 00000000 00577db8 c0000417 kernel32!UnhandledExceptionFilter+0x1af
08 0812be3c 630ec3b4 00000002 00000000 630ec300 ucrtbase!__acrt_call_reportfault+0x110
09 0812be48 630ec300 00000000 00000000 00000000 ucrtbase!_invoke_watson+0x24
0a 0812be78 630ec35b 6309952a 00000009 b2da496e ucrtbase!_invalid_parameter+0xb1
0b 0812be8c 630a1f3d 0de96e10 0812c108 61ad1b06 ucrtbase!_invalid_parameter_noinfo+0xb
0c 0812be98 61ad1b06 0812bec4 00000104 00582358 ucrtbase!strcpy_s+0x2d
0d (Inline) -------- -------- -------- -------- usvfs_x86!strcpy_s+0x13 [c:\program files (x86)\windows kits\10\include\10.0.16299.0\ucrt\string.h @ 123] 
0e 0812c160 61aca769 0812c880 0812c96c 0812c8fc usvfs_x86!RerouteW::create+0x2a6 [e:\mo2\build_32\usvfs\usvfs\hooks\kernel32.cpp @ 140] 
0f 0812c988 40000923 0a6de3d8 00000000 0812c9b8 usvfs_x86!usvfs::hooks::FindFirstFileExW+0xc9 [e:\mo2\build_32\usvfs\usvfs\hooks\kernel32.cpp @ 1607]
...

The problem is that strcpy_s is called with a source string which is larger than the destination size of 260 bytes. The source string is:

da 00582358
00582358  "C:\Program Files (x86)\Steam\ste"
00582378  "amapps\common\Skyrim Special Edi"
00582398  "tion\Data\tools\GenerateFNIS_for"
005823b8  "_Users\.vs\.vs\.vs\.vs\.vs\.vs\."
005823d8  "vs\.vs\.vs\.vs\.vs\.vs\.vs\.vs\."
005823f8  "vs\.vs\.vs\.vs\.vs\.vs\.vs\.vs\."
00582418  "vs\.vs\.vs\.vs\.vs\.vs\.vs\.vs\."
00582438  "vs\.vs\.vs\.vs\.vs\.vs\.vs\.vs\."
00582458  "vs\.vs"

IMHO strcpy_s is an evil function because when the destination is too small it terminates your program in a very ungraceful manner. Usually the strncpy_s with _TRUNCATE is what you actually want.

Silarn commented 6 years ago

I probably know where this is happening. I can look into solving it later.

erasmux commented 6 years ago

I have fixed this on my private build. Will make a pull request soon.

Silarn commented 6 years ago

Merged and I think fixed? Going to close this. If for some reason it isn't fixed we can reopen.