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.

Sign up for GitHub

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

Jump to bottom

Heap buffer overflow crash in Longtail_GetFilePermissions #211

Closed
timsjostrand opened this issue Nov 18, 2022 · 4 comments · Fixed by #212 or DanEngelbrecht/golongtail#235
Closed

Heap buffer overflow crash in Longtail_GetFilePermissions #211

timsjostrand opened this issue Nov 18, 2022 · 4 comments · Fixed by #212 or DanEngelbrecht/golongtail#235
Labels
bug Something isn't working

Comments

@timsjostrand
Copy link
Contributor

Hi! Our studio has been using Longtail 0.3.6 to successfully deploy 100s of software packages/versions and TBs of data. Thanks for the great work!

Recently, a newly deployed package started crashing our in-house Launcher application (a standalone Slate app that wraps the C longtail lib in an Unreal C++ plugin).

I have been debugging this issue for a couple of days and something in the Long Path handling seems to crash. This is reproducible on every downsync of this particular package into this particular target directory. Specifying a shorter target directory avoids the crash -- presumably because the long path patching code path is never reached.

I am sure that syncing long paths has worked in the past however, so I am still a little confused.

The crash happens on Longtail_Free in Longtail_GetFilePermissions:

    int Longtail_GetFilePermissions(const char* path, uint16_t* out_permissions)
    {
    #if defined(LONGTAIL_ASSERTS)
        MAKE_LOG_CONTEXT_FIELDS(ctx)
            LONGTAIL_LOGFIELD(path, "%s"),
            LONGTAIL_LOGFIELD(out_permissions, "%p")
        MAKE_LOG_CONTEXT_WITH_FIELDS(ctx, 0, LONGTAIL_LOG_LEVEL_OFF)
    #else
        struct Longtail_LogContextFmt_Private* ctx = 0;
    #endif // defined(LONGTAIL_ASSERTS)

        wchar_t* long_path = MakeLongPlatformPath(path);
        DWORD attrs = GetFileAttributesW(long_path);
        Longtail_Free(long_path);
        ^^^
        if (attrs == INVALID_FILE_ATTRIBUTES)
[...]

Expected long_path in the crashing call stack:

\\?\C:\Users\tim.sjostrand\AppData\Local\Thunderful Games\UE\Engine\Binaries\ThirdParty\DotNet\6.0.302\windows\sdk\6.0.302\DotnetTools\dotnet-watch\6.0.302-servicing.22323.12\tools\net6.0\any\runtimes\win\lib\netstandard2.0\System.Security.Cryptography.ProtectedData.dll

Inspecting long_path in the VS debugger (for whatever that is worth) reveals the following, shorter, string:

\\?\C:\Users\tim.sjostrand\AppData\Local\Thunderful Games\UE\Engine\Binaries\ThirdParty\DotNet\6.0.302\windows\sdk\6.0.302\DotnetTools\dotnet-watch\6.0.302-serv

AddressSanitizer hints at MakeLongPlatformPath being the culprit:

==46372==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x11ef3c6c364f at pc 0x7ff970d130b1 bp 0x0068514cf550 sp 0x0068514cf568
READ of size 532 at 0x11ef3c6c364f thread T98
    #0 0x7ff970d130e0 in _asan_wrap_GlobalSize+0x4026d (D:\Work\Launcher\Launcher\Binaries\Win64\clang_rt.asan_dynamic-x86_64.dll+0x1800430e0)
    #1 0x7ff9c4869351 in RtlAppendUnicodeStringToString+0x51 (C:\WINDOWS\SYSTEM32\ntdll.dll+0x180049351)
    #2 0x7ff9c486924d in RtlCheckTokenMembershipEx+0x3ad (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18004924d)
    #3 0x7ff9c4866915 in RtlFindActivationContextSectionString+0xd75 (C:\WINDOWS\SYSTEM32\ntdll.dll+0x180046915)
    #4 0x7ff9c4864822 in RtlDosSearchPath_Ustr+0xbe2 (C:\WINDOWS\SYSTEM32\ntdll.dll+0x180044822)
    #5 0x7ff9c489e848 in RtlDosPathNameToNtPathName_U_WithStatus+0x18 (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18007e848)
    #6 0x7ff9c2035928 in GetFileAttributesW+0x38 (C:\WINDOWS\System32\KERNELBASE.dll+0x180035928)
    #7 0x7ff637aa0dc4 in Longtail_GetFilePermissions D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\lib\longtail_platform.c:881
    #8 0x7ff637aa99be in FSStorageAPI_GetPermissions D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\lib\filestorage\longtail_filestorage.c:257
    #9 0x7ff637a9e60f in WriteAssetsFromBlock D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\src\longtail.c:5183
    #10 0x7ff637aa4130 in Bikeshed_Job D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\lib\bikeshed\longtail_bikeshed.c:244
    #11 0x7ff637aa54e5 in ThreadWorker_Execute D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\lib\bikeshed\longtail_bikeshed.c:92
    #12 0x7ff970d2da49 in _asan_wrap_GlobalSize+0x5abd6 (D:\Work\Launcher\Launcher\Binaries\Win64\clang_rt.asan_dynamic-x86_64.dll+0x18005da49)
    #13 0x7ff9c323554f in BaseThreadInitThunk+0xf (C:\WINDOWS\System32\KERNEL32.DLL+0x18001554f)
    #14 0x7ff9c482485a in RtlUserThreadStart+0x2a (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18000485a)

0x11ef3c6c364f is located 0 bytes to the right of 271-byte region [0x11ef3c6c3540,0x11ef3c6c364f)
allocated by thread T98 here:
    #0 0x7ff970d1f9b2 in _asan_wrap_GlobalSize+0x4cb3f (D:\Work\Launcher\Launcher\Binaries\Win64\clang_rt.asan_dynamic-x86_64.dll+0x18004f9b2)
    #1 0x7ff637a8e9b4 in Longtail_Alloc D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\src\longtail.c:765
    #2 0x7ff637aa200d in MakeLongPlatformPath D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\lib\longtail_platform.c:392
    #3 0x7ff637aa0db8 in Longtail_GetFilePermissions D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\lib\longtail_platform.c:880
    #4 0x7ff637aa99be in FSStorageAPI_GetPermissions D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\lib\filestorage\longtail_filestorage.c:257
    #5 0x7ff637a9e60f in WriteAssetsFromBlock D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\src\longtail.c:5183
    #6 0x7ff637aa4130 in Bikeshed_Job D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\lib\bikeshed\longtail_bikeshed.c:244
    #7 0x7ff637aa54e5 in ThreadWorker_Execute D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\lib\bikeshed\longtail_bikeshed.c:92
    #8 0x7ff970d2da49 in _asan_wrap_GlobalSize+0x5abd6 (D:\Work\Launcher\Launcher\Binaries\Win64\clang_rt.asan_dynamic-x86_64.dll+0x18005da49)
    #9 0x7ff9c323554f in BaseThreadInitThunk+0xf (C:\WINDOWS\System32\KERNEL32.DLL+0x18001554f)
    #10 0x7ff9c482485a in RtlUserThreadStart+0x2a (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18000485a)

Thread T98 created by T13 here:
    #0 0x7ff970d2f118 in _asan_wrap_GlobalSize+0x5c2a5 (D:\Work\Launcher\Launcher\Binaries\Win64\clang_rt.asan_dynamic-x86_64.dll+0x18005f118)
    #1 0x7ff637aa0b0a in Longtail_CreateThread D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\lib\longtail_platform.c:203
    #2 0x7ff637aa4038 in Bikeshed_Init D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\lib\bikeshed\longtail_bikeshed.c:639
    #3 0x7ff637aa4e73 in Longtail_CreateBikeshedJobAPI D:\Work\Launcher\Launcher\Plugins\Longtail\Source\LongtailLib\0.3.6\lib\bikeshed\longtail_bikeshed.c:672
    #4 0x7ff637a733e1 in Longtail::Downsync D:\Work\Launcher\Launcher\Plugins\Longtail\Source\Longtail\Private\Longtail.cpp:353
    #5 0x7ff637a72cab in FLongtailModule::Downsync D:\Work\Launcher\Launcher\Plugins\Longtail\Source\Longtail\Private\Longtail.cpp:674
    #6 0x7ff633cdb3a3 in Launcher::InstallSync D:\Work\Launcher\Launcher\Source\Launcher\Private\OperationQueue\InstallOp.cpp:329
    #7 0x7ff633cd7a99 in Launcher::InstallOp D:\Work\Launcher\Launcher\Source\Launcher\Private\OperationQueue\InstallOp.cpp:403
    #8 0x7ff633cccda6 in <lambda_ed5755f0d13208a9e86265d7a3a5d932>::operator() D:\Work\Launcher\Launcher\Source\Launcher\Private\OperationQueue\InstallOp.cpp:568
    #9 0x7ff633cd1253 in UE::Core::Private::Function::TFunctionRefCaller<<lambda_ed5755f0d13208a9e86265d7a3a5d932>,TOptional<Launcher::FOpError> __cdecl(TSharedPtr<Launcher::FOperationQueue,1>,Launcher::FOpState &)>::Call D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Public\Templates\Function.h:465
    #10 0x7ff63374e53b in UE::Core::Private::Function::TFunctionRefBase<UE::Core::Private::Function::TFunctionStorage<1>,TOptional<Launcher::FOpError> __cdecl(TSharedPtr<Launcher::FOperationQueue,1>,Launcher::FOpState &)>::operator() D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Public\Templates\Function.h:602
    #11 0x7ff63374dc94 in <lambda_9609f301bf7f1a1286aad3a32d27f010>::operator() D:\Work\Launcher\Launcher\Source\Launcher\Private\OperationQueue\OperationQueue.cpp:85
    #12 0x7ff633750a32 in UE::Core::Private::Function::TFunctionRefCaller<<lambda_9609f301bf7f1a1286aad3a32d27f010>,void __cdecl(void)>::Call D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Public\Templates\Function.h:474
    #13 0x7ff633cfaca2 in TGraphTask<FAsyncGraphTask>::ExecuteTask D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Public\Async\TaskGraphInterfaces.h:1348
    #14 0x7ff633d05b8c in <lambda_17c904c32264d0348d15245fba0e1bff>::operator() D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Public\Async\Fundamental\Task.h:504
    #15 0x7ff633d0c7b2 in LowLevelTasks::TTaskDelegate<LowLevelTasks::FTask * __cdecl(bool),48>::TTaskDelegateImpl<<lambda_17c904c32264d0348d15245fba0e1bff>,0>::CallAndMove D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Public\Async\Fundamental\TaskDelegate.h:171
    #16 0x7ff633626527 in LowLevelTasks::FTask::ExecuteTask D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Public\Async\Fundamental\Task.h:656
    #17 0x7ff633d31d94 in LowLevelTasks::FScheduler::ExecuteTask D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\Async\Fundamental\Scheduler.cpp:179
    #18 0x7ff633d2d341 in LowLevelTasks::FScheduler::TryExecuteTaskFrom<&LowLevelTasks::TLocalQueueRegistry<1024>::TLocalQueue::DequeueGlobal,0> D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\Async\Fundamental\Scheduler.cpp:360
    #19 0x7ff633d363c1 in LowLevelTasks::FScheduler::WorkerMain D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\Async\Fundamental\Scheduler.cpp:402
    #20 0x7ff633d308d2 in UE::Core::Private::Function::TFunctionRefCaller<<lambda_3cce1c92894d9b7915d2a1e93599b8f1>,void __cdecl(void)>::Call D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Public\Templates\Function.h:474
    #21 0x7ff63371af44 in UE::Core::Private::Function::TFunctionRefBase<UE::Core::Private::Function::TFunctionStorage<1>,void __cdecl(void)>::operator() D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Public\Templates\Function.h:602
    #22 0x7ff633fb2b7c in FThreadImpl::Run D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\HAL\Thread.cpp:67
    #23 0x7ff634bbb9cc in FRunnableThreadWin::Run D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\Windows\WindowsRunnableThread.cpp:146
    #24 0x7ff634bbb81a in FRunnableThreadWin::GuardedRun D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\Windows\WindowsRunnableThread.cpp:71
    #25 0x7ff970d2da49 in _asan_wrap_GlobalSize+0x5abd6 (D:\Work\Launcher\Launcher\Binaries\Win64\clang_rt.asan_dynamic-x86_64.dll+0x18005da49)
    #26 0x7ff9c323554f in BaseThreadInitThunk+0xf (C:\WINDOWS\System32\KERNEL32.DLL+0x18001554f)
    #27 0x7ff9c482485a in RtlUserThreadStart+0x2a (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18000485a)

Thread T13 created by T0 here:
    #0 0x7ff970d2f118 in _asan_wrap_GlobalSize+0x5c2a5 (D:\Work\Launcher\Launcher\Binaries\Win64\clang_rt.asan_dynamic-x86_64.dll+0x18005f118)
    #1 0x7ff634ba719c in FRunnableThreadWin::CreateInternal D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\Windows\WindowsRunnableThread.h:166
    #2 0x7ff633fcb26f in FRunnableThread::SetupCreatedThread D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\HAL\ThreadingBase.cpp:824
    #3 0x7ff633fbfd66 in FRunnableThread::Create D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\HAL\ThreadingBase.cpp:815
    #4 0x7ff633fb198f in FThreadImpl::FThreadImpl D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\HAL\Thread.cpp:29
    #5 0x7ff633fb0f2a in SharedPointerInternals::TIntrusiveReferenceController<FThreadImpl,1>::TIntrusiveReferenceController<FThreadImpl,1><wchar_t const * &,TUniqueFunction<void __cdecl(void)>,<lambda_fdbfd329f7733661909bd1c3e9bf4728>,unsigned int &,enum EThreadPriority &,FThreadAffinity &,enum FThread::EForkable &> D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Public\Templates\SharedPointerInternals.h:394
    #6 0x7ff633fb1684 in FThread::FThread D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\HAL\Thread.cpp:118
    #7 0x7ff633d30da1 in LowLevelTasks::FScheduler::CreateWorker D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\Async\Fundamental\Scheduler.cpp:85
    #8 0x7ff633d34dfa in LowLevelTasks::FScheduler::StartWorkers D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\Async\Fundamental\Scheduler.cpp:148
    #9 0x7ff633d01c82 in FTaskGraphCompatibilityImplementation::FTaskGraphCompatibilityImplementation D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\Async\TaskGraph.cpp:1922
    #10 0x7ff633d201fa in FTaskGraphInterface::Startup D:\Work\Launcher\Engine\Engine\Source\Runtime\Core\Private\Async\TaskGraph.cpp:2450
    #11 0x7ff633648a0f in FEngineLoop::PreInitPreStartupScreen D:\Work\Launcher\Engine\Engine\Source\Runtime\Launch\Private\LaunchEngineLoop.cpp:2289
    #12 0x7ff63364051c in FEngineLoop::PreInit D:\Work\Launcher\Engine\Engine\Source\Runtime\Launch\Private\LaunchEngineLoop.cpp:3939
    #13 0x7ff6336591b4 in RunLauncher D:\Work\Launcher\Launcher\Source\Launcher\Private\LauncherApp.cpp:261
    #14 0x7ff633cb8445 in WinMain D:\Work\Launcher\Launcher\Source\Launcher\Private\Windows\LauncherMainWindows.cpp:8
    #15 0x7ff63822bab9 in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #16 0x7ff9c323554f in BaseThreadInitThunk+0xf (C:\WINDOWS\System32\KERNEL32.DLL+0x18001554f)
    #17 0x7ff9c482485a in RtlUserThreadStart+0x2a (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18000485a)

SUMMARY: AddressSanitizer: heap-buffer-overflow (D:\Work\Launcher\Launcher\Binaries\Win64\clang_rt.asan_dynamic-x86_64.dll+0x1800430e0) in _asan_wrap_GlobalSize+0x4026d
Shadow bytes around the buggy address:
  0x040923758670: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x040923758680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x040923758690: 00 00 00 00 00 00 00 00 00 00 04 fa fa fa fa fa
  0x0409237586a0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0409237586b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0409237586c0: 00 00 00 00 00 00 00 00 00[07]fa fa fa fa fa fa
  0x0409237586d0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0409237586e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0409237586f0: 00 00 00 00 00 00 00 00 00 03 fa fa fa fa fa fa
  0x040923758700: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x040923758710: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
Address Sanitizer Error: Heap buffer overflow

I hope to be able to take a deeper dive into the issue soon, but thought I would leave this information here in public until that time.

In the interim the crash can be worked around by having the application make sure the target path is always prefixed with \\?\ before calling any longtail APIs -- this seems to avoid the per-file path patching code altogether.
@DanEngelbrecht
Copy link
Owner

Seems iffy, I'll see if I can get some time to debug.
I think possibly we need to check if long_path != path to see if we should free the string, but will need to dig a little deeper.

@DanEngelbrecht
Copy link
Owner

In the interim the crash can be worked around by having the application make sure the target path is always prefixed with \?\ before calling any longtail APIs -- this seems to avoid the per-file path patching code altogether.

If you prefix with \\?\ the logic to turn it into a long path if the path exceeds a certain length (which you example does) longtail will not try to do prefixing itself.

@DanEngelbrecht DanEngelbrecht mentioned this issue Nov 21, 2022
Merged
@DanEngelbrecht
Copy link
Owner

Found the issue, see #212
Silly error when allocating string buffer.

@DanEngelbrecht DanEngelbrecht added the bug Something isn't working label Nov 22, 2022
@timsjostrand
Copy link
Contributor Author

Yes that does look correct. Thanks for the fast fix! 🙏

@DanEngelbrecht DanEngelbrecht mentioned this issue Nov 26, 2022
Merged
@timsjostrand timsjostrand mentioned this issue May 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
2 participants
@timsjostrand @DanEngelbrecht