Git: "Memory violation detected" by Dell Data Protection
Setup
- Git version
$ git --version --build-options
git version 2.17.1.windows.2
cpu: x86_64
built from commit: a60968cf435951d9411fc0f980a2e362d5cccea2
sizeof-long: 4
- Running Windows 7
$ cmd.exe /c ver
Microsoft Windows [Version 6.1.7601]
- Install options
$ type "C:\Program Files\Git\etc\install-options.txt"
Editor Option: Notepad++
Path Option: Cmd
SSH Option: OpenSSH
CURL Option: OpenSSL
CRLF Option: CRLFCommitAsIs
Bash Terminal Option: MinTTY
Performance Tweaks FSCache: Enabled
Use Credential Manager: Enabled
Enable Symlinks: Disabled
- Any other interesting things about your environment that might be related
to the issue you're seeing?
Running Dell Data Protection for disk encryption and "advanced threat detection".
Details
Let me start by saying that I don't know if this is a real problem in Git for Windows or a false positive from Dell Data Protection, but either way I would love to find a fix. For the last several releases of Git for Windows (at least), I've noticed multi-second delays while doing some basic operations. My company uses Dell Data Protection on all of our machines, and I'm pretty sure it is causing the delays.
The most trivial thing that triggers a delay is simply opening Git Bash. When I do that, Dell Data Protection's log gets new messages like these:
2018.06.13 14:20:19 I AdvATP : Processing Agent event MemViolationDetected Warning
'{
"message":"Memory Violation detected : {0} {1} {2} {3} {4}",
"machinename":"Fred",
"path":"C:\\Program Files\\Git\\usr\\bin\\mintty.exe",
"sha256":"4C21AF9E43FE00E4A3C242ED856305D44B379BEB4D80FFE49786EC9F9389C44A",
"processid":"19768",
"violationType":"2"
}' (Escrowed)
2018.06.13 14:20:24 I AdvATP : Processing Agent event MemViolationDetected Warning
'{
"message":"Memory Violation detected : {0} {1} {2} {3} {4}",
"machinename":"Fred",
"path":"C:\\Program Files\\Git\\usr\\bin\\bash.exe",
"sha256":"FC98A4CB037259E59C908778446FA4738087F3381547CAFDC9AE132819320585",
"processid":"6448",
"violationType":"1"
}' (Escrowed)
This is for Git for Windows version 2.17.1.windows.2. Sometimes I also see sh.exe in the log, but I'm not sure what operation causes that and sh.exe has the same hash as bash.exe anyway. I do not know what "violationType" 1 or 2 means.
My only workaround is to very politely ask IT to whitelist the hash values of these exes, but those may change with every Git for Windows release so it's not a great solution. How can I dig into this a bit more to at least figure out if the problem is in Git for Windows or Dell Data Protection?
shoelzer
All 28 comments
How can I dig into this a bit more to at least figure out if the problem is in Git for Windows or Dell Data Protection?
I would also love to know the answer to that question.
dscho
on 15 Jun 2018
I would do this:
- install Git-for-Windows SDK
- run c:\git-sdk-64\msys2_shell.cmd (not git-bash because it will add /mingw64/bin/gcc to the path which is used to compile native Windows executables, but we need /usr/bin/gcc which links against msys-2.0.dll)
- install base-devel
- clone https://github.com/git-for-windows/MSYS2-packages
- build bash with makepkg -s
- build bash without makepkg (use PKGBUILD as a reference); typically you can just cd to the source and run make
- remove lots of stuff from bash source code (think "bisect the source")
- build bash again and try if you can reproduce the issue with this executable
- put stuff back in if the issue goes away
- repeat 7-9 until you have narrowed it down
There is no guarantee that this will work since we cannot learn much about Dell Data Protection directly.
drizzd
on 20 Jun 2018
I can tell you that this most likely is NOT a false-positive. I don't have the Dell tool, but if you watch in the debugger and set a breakpoint on EnumFontFamiliesExW, you will see that the callback address points to code on the call stack! There is a preceding call to VirtualProtect that changes the call stack to PAGE_EXECUTE_READWRITE.
That is extremely dangerous and dumb!
x64 code:
3: kd> k
# Child-SP RetAddr Call Site
00 EnumFontFamiliesExW (note: I wrote code that hooked this function in order to test that the callback is within the stack)
01 00000000`ffffc0c0 00000001`00431f61 mintty+0x305e9
02 00000000`ffffc370 00000001`0043799c mintty+0x31f61
03 00000000`ffffc490 00000001`80047f24 mintty+0x3799c
04 00000000`ffffcac0 00000001`80045a03 msys_2_0!msys_dll_init+0x1234
05 00000000`ffffcd70 00000001`80045ab4 msys_2_0!setprogname+0x33b3
06 00000000`ffffcdd0 00000000`00000000 msys_2_0!setprogname+0x3464
3: kd> dv
hDC = 0xffffffff`9e010cf1
pLogFont = 0x00000000`ffffc220
pfnEnumFontFamExProc = 0x00000000`ffffc1ff <-- This is the callback passed to EnumFontFamiliesExW
lParam = 0n0
dwFlags = 0
ulpLowStack = 0xffe00000
ulpCallback = 0xffffc1ff
ulpHighStack = 0x00000001`00000000
Here is the code written to the stack:
3: kd> u 0x00000000`ffffc1ff
00000000`ffffc1ff 49bba0f9420001000000 mov r11,offset mintty+0x2f9a0 (00000001`0042f9a0)
00000000`ffffc209 49bae0c1ffff00000000 mov r10,0FFFFC1E0h
00000000`ffffc213 49ffe3 jmp r11
00000000`ffffc216 90 nop
00000000`ffffc217 0070c3 add byte ptr [rax-3Dh],dh
And here you can see that it falls within the Stack Limits:
3: kd> !teb
TEB at 00000000003bd000
ExceptionList: 0000000000000000
StackBase: 0000000100000000
StackLimit: 00000000fffc8000
SubSystemTib: 0000000000000000
.
.
.
As mentioned, an earlier breakpoint on VirtualProtect shows that the stack is altered to be PAGE_EXECUTE_READWRITE (0x40). This make MinTTY extremely vulnerable. Especially when you realize that RSP within EnumFontFamiliesExW is very close to the address of the callback. A small overflow and you can change that code to gain execution!
alsliahona
on 22 Jun 2018
Do by any chance you have an AV or anti-malware solution installed on your machine as well?
Then this commit in MSYS2 might fix your problem: https://github.com/git-for-windows/msys2-runtime/commit/36e86e52805541f284fdab489821d6571f5806d0 however, I have no clue when it will be integrated in a Git for Windows release.
See also on the cygwin mailing list for more information: https://cygwin.com/ml/cygwin-patches/2018-q2/msg00016.html
Related to #1630?
jcberthon
on 22 Aug 2018
@jcberthon AFAICT it is only in the pre-release v2.19.0-rc0 so far.
dscho
on 22 Aug 2018
@jcberthon Thanks for the info. I don't have separate anti-malware running, but Dell Data Protection does both disk encryption and "advanced threat detection".
I downloaded "PortableGit-2.19.0.rc0.windows.1-64-bit.7z.exe" and ran git-bash.exe. I got the same type of warnings in Dell Data Protection's log so unfortunately that cygwin/MSYS2 change doesn't seem to fix my issue.
shoelzer
on 23 Aug 2018
I can tell you that this most likely is NOT a false-positive.
@alsliahona do you see the same issue in Cygwin? In MSYS2?
dscho
on 28 Nov 2018
Quick update from the guy who opened this issue: My company stopped using Dell Data Protection and I'm not having any issues with the new anti-virus software. Hurrah!
Unfortunately this also means that I don't have a way to test potential fixes so maybe this issue should be closed. I'll leave that decision up to @dscho.
shoelzer
on 28 Nov 2018
I can tell you that this most likely is NOT a false-positive.
@alsliahona do you see the same issue in Cygwin? In MSYS2?
FWIW, the root cause of the problem was reported to oss-security back in October. To my knowledge the bug has not been fixed yet.
On the chance that this information did not reach the git-for-windows devs, the text of the disclosure follows:
Introduction to GCC Compiler Induced Vulnerability
==================================================
Hal Lonas
11 October 2018
INTRODUCTION
Webroot engineers recently discovered a vulnerability with Linux and Windows
executables produced by the Gnu C Compiler, commonly known as GCC.
Technical Description of the vulnerability
When nested C functions are compiled by GCC, code is generated which causes the
call stack of the currently executing thread to be made executable prior to the
call to a nested function and for the duration of the thread’s lifetime. This
is essentially the equivalent of disabling Data Execution Prevention (DEP).
A stack overflow, etc., that is able to place instructions on the page(s) of
memory made executable has the potential of gaining execution and running
malware, etc. This places the process at substantial risk of being exploited.
How was the vulnerability found?
Engineers using anti-exploit tools developed at Webroot found this
vulnerability in commonly used tools such as:
• Git for Windows Installer
• Cygwin Installer
• MinTTY
• Git Bash Shell
• …and other similar tools
What versions of GCC have we tested?
We have found the vulnerability to be produced when using the following
versions of GCC:
• 8.1
• 7.3
• 7.1
These were the only versions we tested and all produced the vulnerability in
output executables. No other GCC versions were tested.
Why this communication?
We are taking this opportunity to inform the custodians of GCC so that the
vulnerability might be addressed before it becomes public knowledge.
Will Webroot communicate this to the public?
Webroot believes in responsible disclosure and will work with third parties to
ensure that the vulnerability is addressed before a public announcement. We
are happy to work with your communications team on announcement timing.
==============================================================================
DETAILED DISCLOSURE FOLLOWS
==============================================================================
Webroot Security Vulnerability Disclosure
=========================================
Software compiled with various versions of GCC on Windows and Linux may contain a serious security vulnerability. The
vulnerability will exist when C code with nested functions are compiled. Examples of vulnerable software include Cygwin
Bash, MinTTY, and similar tools included with Git for Windows, and other Unix-like tools on Windows, etc.
On x86 / x64 Linux based systems (and possibly other Unix systems) any tool compiled with GCC which utilizes nested C
functions is vulnerable.
Vulnerability
=============
When nested C functions are compiled by GCC, code is generated which causes the call stack of the currently executing
thread to be made executable prior to the call to a nested function and for the duration of the thread’s lifetime.
This is essentially the equivalent of disabling Data Execution Prevention (DEP). A stack overflow, etc., that is able
to place instructions on the page(s) of memory made executable has the potential of gaining execution and running
malware, etc. This places the process at substantial risk of being exploited.
Windows Example
===============
The following simple C program, when compiled by GCC, generates code that has an executable stack shortly after main()
is entered:
#include <stdio.h>
#include <Windows.h>
int main()
{
BOOL CALLBACK EnumWindowsCB(HWND hWnd, LPARAM lp)
{
printf("Window: %p\n", hWnd);
}
printf("Enum'd Windows:\n");
EnumWindows(EnumWindowsCB, 0);
return 0;
}
When compiled as an x86_64 binary, main looks like this:
.text:000000000040157B ; =============== S U B R O U T I N E =======================================
.text:000000000040157B
.text:000000000040157B ; Attributes: bp-based frame
.text:000000000040157B
.text:000000000040157B ; int __cdecl main(int argc, const char **argv, const char **envp)
.text:000000000040157B public main
.text:000000000040157B main proc near ; CODE XREF: __tmainCRTStartup+242p
.text:000000000040157B ; DATA XREF: .pdata:000000000040506Co ...
.text:000000000040157B
.text:000000000040157B var_30 = byte ptr -30h
.text:000000000040157B var_10 = qword ptr -10h
.text:000000000040157B arg_0 = byte ptr 10h
.text:000000000040157B
.text:000000000040157B push rbp
.text:000000000040157C mov rbp, rsp
.text:000000000040157F sub rsp, 50h
.text:0000000000401583 call __main
.text:0000000000401588 lea rax, [rbp+arg_0]
.text:000000000040158C mov [rbp+var_10], rax
.text:0000000000401590 lea rax, [rbp+var_30]
.text:0000000000401594 lea rdx, [rbp+var_30]
.text:0000000000401598 mov word ptr [rax], 0BB49h
.text:000000000040159D lea rcx, EnumWindowsCB_84527
.text:00000000004015A4 mov [rax+2], rcx
.text:00000000004015A8 mov word ptr [rax+0Ah], 0BA49h
.text:00000000004015AE mov [rax+0Ch], rdx
.text:00000000004015B2 mov dword ptr [rax+14h], 90E3FF49h
.text:00000000004015B9 mov rcx, rax
.text:00000000004015BC call __enable_execute_stack
.text:00000000004015C1 lea rcx, aEnumDWindows ; "Enum'd Windows:"
.text:00000000004015C8 call puts
.text:00000000004015CD lea rax, [rbp+var_30]
.text:00000000004015D1 mov edx, 0
.text:00000000004015D6 mov rcx, rax
.text:00000000004015D9 mov rax, cs:__imp_EnumWindows
.text:00000000004015E0 call rax ; __imp_EnumWindows
.text:00000000004015E2 mov eax, 0
.text:00000000004015E7 add rsp, 50h
.text:00000000004015EB pop rbp
.text:00000000004015EC retn
.text:00000000004015EC main endp
.text:00000000004015EC
.text:00000000004015EC ; ---------------------------------------------------------------------------
The nested function “EnumWindowsCB” is referenced in the lea instruction at address 40159D. It looks like this (which
is essentially the same as it would look as a non-nested function):
.text:0000000000401550 ; =============== S U B R O U T I N E =======================================
.text:0000000000401550
.text:0000000000401550 ; Attributes: bp-based frame
.text:0000000000401550
.text:0000000000401550 EnumWindowsCB_84527 proc near ; DATA XREF: main+22o
.text:0000000000401550 ; .pdata:000000000040506Co
.text:0000000000401550
.text:0000000000401550 var_8 = qword ptr -8
.text:0000000000401550 arg_0 = qword ptr 10h
.text:0000000000401550 arg_8 = qword ptr 18h
.text:0000000000401550
.text:0000000000401550 push rbp
.text:0000000000401551 mov rbp, rsp
.text:0000000000401554 sub rsp, 30h
.text:0000000000401558 mov [rbp+arg_0], rcx
.text:000000000040155C mov [rbp+arg_8], rdx
.text:0000000000401560 mov [rbp+var_8], r10
.text:0000000000401564 mov rdx, [rbp+arg_0]
.text:0000000000401568 lea rcx, aWindowP ; "Window: %p\n"
.text:000000000040156F call printf
.text:0000000000401574 nop
.text:0000000000401575 add rsp, 30h
.text:0000000000401579 pop rbp
.text:000000000040157A retn
.text:000000000040157A EnumWindowsCB_84527 endp
Despite the fact that EnumWindowsCB does not need to access any local variables in main(), the code in main() between
401590 and 4015BC sets up stack variables that would make this possible, and then the CALL at address 4015BC makes the
call stack itself at least partially executable, by passing the address of the context structure [var_30] to
__enable_execute_stack, which looks like this:
.text:0000000000402AB0 ; =============== S U B R O U T I N E =======================================
.text:0000000000402AB0
.text:0000000000402AB0
.text:0000000000402AB0 public __enable_execute_stack
.text:0000000000402AB0 __enable_execute_stack proc near ; CODE XREF: main+41p
.text:0000000000402AB0 ; DATA XREF: .pdata:0000000000405228o
.text:0000000000402AB0
.text:0000000000402AB0 dwLength = qword ptr -38h
.text:0000000000402AB0 flNewProtect = dword ptr -20h
.text:0000000000402AB0
.text:0000000000402AB0 push rbx
.text:0000000000402AB1 sub rsp, 50h
.text:0000000000402AB5 mov r8d, 30h
.text:0000000000402ABB lea rbx, [rsp+58h+dwLength]
.text:0000000000402AC0 mov rdx, rbx ; dwLength
.text:0000000000402AC3 call cs:__imp_VirtualQuery
.text:0000000000402AC9 test rax, rax
.text:0000000000402ACC jz __enable_execute_stack_cold_0
.text:0000000000402AD2 mov rdx, qword ptr [rsp+58h+flNewProtect] ; flNewProtect
.text:0000000000402AD7 lea r9, [rbx+24h]
.text:0000000000402ADB mov r8d, 40h
.text:0000000000402AE1 mov rcx, [rsp+58h+dwLength] ; lpflOldProtect
.text:0000000000402AE6 call cs:__imp_VirtualProtect
.text:0000000000402AEC nop
.text:0000000000402AED add rsp, 50h
.text:0000000000402AF1 pop rbx
.text:0000000000402AF2 retn
.text:0000000000402AF2 __enable_execute_stack endp
The code in __enable_execute_stack() calls VirtualQuery() to find out the RegionSize and the BaseAddress of the
structure [var_30]. It then calls VirtualProtect to make this entire region PAGE_EXECUTE_READWRITE. At a minimum
one whole page (0x1000 bytes) of stack memory is made executable. Potentially many more pages of stack memory could
be made executable by the function, depending upon the results of the call to VirtualQuery (which will return a
RegionSize for all pages from BaseAddress onward that have matching State, Type, and Protect bits). Different
functions are likely to return larger RegionSize results further extending the amount of memory placed at risk.
It is also important to notice that the stack is made executable sometime before the context variable is even used in
the call to EnumWindows() which utilizes the nested C function. This is obvious by the code start at address 4015C1
in main():
.text:00000000004015BC call __enable_execute_stack ; <-- Stack is made executable here <--
.text:00000000004015C1 lea rcx, aEnumDWindows ; "Enum'd Windows:"
.text:00000000004015C8 call puts ; <-- puts definitely does not need an executable stack <--
.text:00000000004015CD lea rax, [rbp+var_30]
.text:00000000004015D1 mov edx, 0
.text:00000000004015D6 mov rcx, rax
.text:00000000004015D9 mov rax, cs:__imp_EnumWindows
.text:00000000004015E0 call rax ; __imp_EnumWindows ; <-- Nested C function called <--
.text:00000000004015E2 mov eax, 0
.text:00000000004015E7 add rsp, 50h
.text:00000000004015EB pop rbp
.text:00000000004015EC retn
.text:00000000004015EC main endp
The call to printf(“Enum’d Windows:\n”) from our code in main() runs AFTER the stack is made executable, but BEFORE
EnumWindows() is called.
This means that not only is the call to EnumWindows() and its (nested) callback function EnumWindowsCB() potentially
capable of intentionally or unintentionally placing exploit instructions or shell code upon the stack, but so also is
every other function called within main(), before or after use of the nested function.
Furthermore, this executable stack memory is leaked as executable. There is no code generated that restores the
original page protections after the nested C function has been utilized for the last time. For the lifetime of the
program, anything that is able to cause a stack overflow (etc.) and cause execution to occur on the stack in the
executable page(s), will not raise an access violation and therefore the process will remain exploitable for the
duration of the current thread.
This flaw in GCC could allow an attacker to gain execution in the same way in which they would if Data Execution
Prevent (DEP) had been disabled on a 32-bit system. Worse, the Windows Task Manager will not show that DEP is
(essentially) disabled, and 64-bit processes (where DEP cannot normally be disabled) are made vulnerable to data
execution by this flaw in GCC generated code.
Note that nested C functions appear to be particular to code compiled with GCC. Most if not all C++ compilers are
able to produce code from lambdas (similar to nested functions) without compromising the call stack.
Linux Example
=============
Below is a similar C program with a nested C function written to run on Linux / Unix:
#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <unistd.h>
//
// DumpMapsStackEntry is a utility function that finds and prints the call stack
// identified by [stack] in the procfs maps file for the current process
void DumpMapsStackEntry()
{
char szMapsFile[1024];
sprintf(&szMapsFile[0], "/proc/%u/maps", getpid());
FILE *pfMaps = fopen(&szMapsFile[0], "rt");
char szLine[1024];
while(NULL != fgets(&szLine[0], sizeof(szLine) - 1, pfMaps))
{
if(NULL == strstr(&szLine[0], "[stack]"))
{
continue;
}
printf("%s\n", &szLine[0]);
}
fclose(pfMaps);
}
//
// EnumerateViaCallback is a "API" that invokes the callback function
void EnumerateViaCallback(void (*pfnCB)(int, const char *),
const char *pszPassThrough)
{
for(int i = 0; i < 10; ++i)
{
pfnCB(i, pszPassThrough);
}
}
#ifndef VULN_TEST_FORCEFULLY_OMIT_CODE
//
// Note: Testing shows that just having this function present causes
// the stack to be executable from main() onward... Even if VulnTest is
// never invoked or even referenced! The ifdef above may be defined
// demonstrate this assertion.
//
void VulnTest(int iTest)
{
if(0 == iTest)
{
printf("Nested Function Omitted\n");
return;
}
//
// Nested C function:
void EnumCallback(int iN, const char *pszPassThrough)
{
printf("N = %i, pass through: %s\n",
iN,
pszPassThrough);
if(5 == iN)
{
printf("Check memory protections of stack pages near %p\n",
__builtin_frame_address(0));
DumpMapsStackEntry();
printf("Press enter to continue\n");
getchar();
}
}
//
// Call the thing that invokes the nested function...
printf("Enumerate 1 - 10\n");
EnumerateViaCallback(EnumCallback, "Test");
}
#endif
int main(int iArgc, const char *ppszArgv[])
{
printf("Current Process: %u\n", getpid());
DumpMapsStackEntry();
#ifdef VULN_TEST_FORCEFULLY_OMIT
printf("Nested function #ifdef'd out!\n");
#else
VulnTest((iArgc > 1) ? 0 : 1);
#endif
printf("Done with callback press Enter to exit\n");
DumpMapsStackEntry();
getchar();
return 0;
}
Though substantially longer than the simple Windows example, this example code is essentially the same other than that
EnumerateViaCallback() was written instead of using a system API that required a callback, and DumpMapsStackEntry() is
called frequently to show whether or not the current call stack is executable.
This code was saved in the file nested.c, and then compiled with three different sets of options as shown below:
• gcc nested.c -o nested_test
• gcc nested.c -o nested_test_ifdefd -DVULN_TEST_FORCEFULLY_OMIT
o This disables the call to VulnTest causing it to be unreferenced
• gcc nested.c -o nested_test_ifdefd_code -DVULN_TEST_FORCEFULLY_OMIT -DVULN_TEST_FORCEFULLY_OMIT_CODE
o This completely removes VulnTest and of course ensures that it is not referenced
The results from running each version are shown below, with the resulting stack memory protections highlighted:
asandoval@ubuntu:~$ gcc nested.c -o nested_test_ifdefd -DVULN_TEST_FORCEFULLY_OMIT
asandoval@ubuntu:~$ gcc nested.c -o nested_test_ifdefd_code -DVULN_TEST_FORCEFULLY_OMIT -DVULN_TEST_FORCEFULLY_OMIT_CODE
asandoval@ubuntu:~$ gcc nested.c -o nested_test
asandoval@ubuntu:~$ ./nested_test
Current Process: 29793
7ffc7cb3f000-7ffc7cb60000 rwxp 00000000 00:00 0 [stack]
Enumerate 1 - 10
N = 0, pass through: Test
N = 1, pass through: Test
N = 2, pass through: Test
N = 3, pass through: Test
N = 4, pass through: Test
N = 5, pass through: Test
Check memory protections of stack pages near 0x7ffc7cb5e2c0
7ffc7cb3f000-7ffc7cb60000 rwxp 00000000 00:00 0 [stack]
Press enter to continue
N = 6, pass through: Test
N = 7, pass through: Test
N = 8, pass through: Test
N = 9, pass through: Test
Done with callback press Enter to exit
7ffc7cb3f000-7ffc7cb60000 rwxp 00000000 00:00 0 [stack]
asandoval@ubuntu:~$ ./nested_test_ifdefd
Current Process: 29794
7ffec66dd000-7ffec66fe000 rwxp 00000000 00:00 0 [stack]
Nested function #ifdef'd out!
Done with callback press Enter to exit
7ffec66dd000-7ffec66fe000 rwxp 00000000 00:00 0 [stack]
asandoval@ubuntu:~$ ./nested_test_ifdefd_code
Current Process: 29796
7ffda444f000-7ffda4470000 rw-p 00000000 00:00 0 [stack]
Nested function #ifdef'd out!
Done with callback press Enter to exit
7ffda444f000-7ffda4470000 rw-p 00000000 00:00 0 [stack]
asandoval@ubuntu:~$
In the first two cases, where the nested C function was present, whether referenced or not, the stack is executable,
making the process vulnerable and essentially disabling DEP for the stack. Only the last instance of the program where
the nested C function is completely compiled out has a non-executable stack.
Additionally, the presence of the nested C function causes the stack to be executable throughout the life of the
program, from start to finish – which is even more risky than the behavior seen on Windows.
The reason for this is evident from the ELF program header for each version of the program. Notice the GNU_STACK
section pointed which is boxed off for highlighting purposes:
asandoval@ubuntu:~$ readelf -l nested_test
Elf file type is DYN (Shared object file)
Entry point 0x7e0
There are 9 program headers, starting at offset 64
Program Headers:
Type Offset VirtAddr PhysAddr
FileSiz MemSiz Flags Align
PHDR 0x0000000000000040 0x0000000000000040 0x0000000000000040
0x00000000000001f8 0x00000000000001f8 R 0x8
INTERP 0x0000000000000238 0x0000000000000238 0x0000000000000238
0x000000000000001c 0x000000000000001c R 0x1
[Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]
LOAD 0x0000000000000000 0x0000000000000000 0x0000000000000000
0x0000000000000eb0 0x0000000000000eb0 R E 0x200000
LOAD 0x0000000000001d70 0x0000000000201d70 0x0000000000201d70
0x00000000000002a0 0x00000000000002a8 RW 0x200000
DYNAMIC 0x0000000000001d80 0x0000000000201d80 0x0000000000201d80
0x00000000000001f0 0x00000000000001f0 RW 0x8
NOTE 0x0000000000000254 0x0000000000000254 0x0000000000000254
0x0000000000000044 0x0000000000000044 R 0x4
GNU_EH_FRAME 0x0000000000000cc8 0x0000000000000cc8 0x0000000000000cc8
0x000000000000005c 0x000000000000005c R 0x4
+----------------------------------------------------------------------------+
| GNU_STACK 0x0000000000000000 0x0000000000000000 0x0000000000000000 |
| 0x0000000000000000 0x0000000000000000 RWE 0x10 |
+----------------------------------------------------------------------------+
GNU_RELRO 0x0000000000001d70 0x0000000000201d70 0x0000000000201d70
0x0000000000000290 0x0000000000000290 R 0x1
Section to Segment mapping:
Segment Sections...
00
01 .interp
02 .interp .note.ABI-tag .note.gnu.build-id .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rela.dyn .rela.plt .init .plt .plt.got .text .fini .rodata .eh_frame_hdr .eh_frame
03 .init_array .fini_array .dynamic .got .data .bss
04 .dynamic
05 .note.ABI-tag .note.gnu.build-id
06 .eh_frame_hdr
07
08 .init_array .fini_array .dynamic .got
asandoval@ubuntu:~$ readelf -l nested_test_ifdefd
Elf file type is DYN (Shared object file)
Entry point 0x7e0
There are 9 program headers, starting at offset 64
Program Headers:
Type Offset VirtAddr PhysAddr
FileSiz MemSiz Flags Align
PHDR 0x0000000000000040 0x0000000000000040 0x0000000000000040
0x00000000000001f8 0x00000000000001f8 R 0x8
INTERP 0x0000000000000238 0x0000000000000238 0x0000000000000238
0x000000000000001c 0x000000000000001c R 0x1
[Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]
LOAD 0x0000000000000000 0x0000000000000000 0x0000000000000000
0x0000000000000ed0 0x0000000000000ed0 R E 0x200000
LOAD 0x0000000000001d70 0x0000000000201d70 0x0000000000201d70
0x00000000000002a0 0x00000000000002a8 RW 0x200000
DYNAMIC 0x0000000000001d80 0x0000000000201d80 0x0000000000201d80
0x00000000000001f0 0x00000000000001f0 RW 0x8
NOTE 0x0000000000000254 0x0000000000000254 0x0000000000000254
0x0000000000000044 0x0000000000000044 R 0x4
GNU_EH_FRAME 0x0000000000000ce8 0x0000000000000ce8 0x0000000000000ce8
0x000000000000005c 0x000000000000005c R 0x4
+---------------------------------------------------------------------------+
| GNU_STACK 0x0000000000000000 0x0000000000000000 0x0000000000000000 |
| 0x0000000000000000 0x0000000000000000 RWE 0x10 |
+---------------------------------------------------------------------------+
GNU_RELRO 0x0000000000001d70 0x0000000000201d70 0x0000000000201d70
0x0000000000000290 0x0000000000000290 R 0x1
Section to Segment mapping:
Segment Sections...
00
01 .interp
02 .interp .note.ABI-tag .note.gnu.build-id .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rela.dyn .rela.plt .init .plt .plt.got .text .fini .rodata .eh_frame_hdr .eh_frame
03 .init_array .fini_array .dynamic .got .data .bss
04 .dynamic
05 .note.ABI-tag .note.gnu.build-id
06 .eh_frame_hdr
07
08 .init_array .fini_array .dynamic .got
asandoval@ubuntu:~$ readelf -l nested_test_ifdefd_code
Elf file type is DYN (Shared object file)
Entry point 0x7e0
There are 9 program headers, starting at offset 64
Program Headers:
Type Offset VirtAddr PhysAddr
FileSiz MemSiz Flags Align
PHDR 0x0000000000000040 0x0000000000000040 0x0000000000000040
0x00000000000001f8 0x00000000000001f8 R 0x8
INTERP 0x0000000000000238 0x0000000000000238 0x0000000000000238
0x000000000000001c 0x000000000000001c R 0x1
[Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]
LOAD 0x0000000000000000 0x0000000000000000 0x0000000000000000
0x0000000000000ce8 0x0000000000000ce8 R E 0x200000
LOAD 0x0000000000000d70 0x0000000000200d70 0x0000000000200d70
0x00000000000002a0 0x00000000000002a8 RW 0x200000
DYNAMIC 0x0000000000000d80 0x0000000000200d80 0x0000000000200d80
0x00000000000001f0 0x00000000000001f0 RW 0x8
NOTE 0x0000000000000254 0x0000000000000254 0x0000000000000254
0x0000000000000044 0x0000000000000044 R 0x4
GNU_EH_FRAME 0x0000000000000b50 0x0000000000000b50 0x0000000000000b50
0x000000000000004c 0x000000000000004c R 0x4
+---------------------------------------------------------------------------+
| GNU_STACK 0x0000000000000000 0x0000000000000000 0x0000000000000000 |
| 0x0000000000000000 0x0000000000000000 RW 0x10 |
+---------------------------------------------------------------------------+
GNU_RELRO 0x0000000000000d70 0x0000000000200d70 0x0000000000200d70
0x0000000000000290 0x0000000000000290 R 0x1
Section to Segment mapping:
Segment Sections...
00
01 .interp
02 .interp .note.ABI-tag .note.gnu.build-id .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rela.dyn .rela.plt .init .plt .plt.got .text .fini .rodata .eh_frame_hdr .eh_frame
03 .init_array .fini_array .dynamic .got .data .bss
04 .dynamic
05 .note.ABI-tag .note.gnu.build-id
06 .eh_frame_hdr
07
08 .init_array .fini_array .dynamic .got
As expected, only the last, instance of the program without the nested C function creates a read-write stack. The
other instances create a vulnerable read-write-execute stack that remains in use for the lifetime of the program.
A script run as an ordinary user can detect the vulnerable programs simply by reading the ELF header.
Versions of GCC Affected
========================
GCC 8.1, 7.3, and 7.1 were tested. Each version generated code with this flaw. No other versions of GCC were tested.
Other versions which support nested C functions are likely to be vulnerable as well.
Many products, including the popular Git for Windows, and Cygwin tools are compiled with GCC versions that produce
vulnerable executables.
Webroot Detection
=================
Webroot Exploit Shield (available only to closed beta participants as of 1 August 2019) detects various forms of stack
exploitation including some forms of Return Oriented Programming (ROP), Stack Pivots, and Stacks being made executable.
Users are warned of such potential exploits in progress and urged to terminate the process when such behavior is
detected. The default behavior of Exploit Shield (absent a customer response) is to terminate processes where a stack
exploit is identified. Currently Webroot Exploit Shield identifies stack exploitation in the following applications
due this flaw in GCC:
• Git for Windows Installer
• Cygwin Installer
• MinTTY
• Git Bash Shell
• and many other similar tools
Research Provided by Andrew Sandoval / Senior Principal Engineer, Webroot Software Inc.
@alsliahona I hope you did not break any embargo posting this. And more importantly: this only describes a vulnerability in GCC, but it does not give me any useful information about reactions from the GCC development team. In particular, a link to a GCC ticket on the GCC bug tracker would be most helpful.
dscho
on 29 Nov 2018
The thread is on the oss-sec mailing list archives at https://seclists.org/oss-sec/2018/q4/82.
It is title/dated a week ago:
GCC Compiler Induced Vulnerability - affects programs compiled with GCC 7 and 8 containing nested functions
From: Andrew Sandoval
Date: Mon, 22 Oct 2018 15:07:55 +0000
PhilipOakley
on 29 Nov 2018
The thread is on the oss-sec mailing list archives at https://seclists.org/oss-sec/2018/q4/82.
It is title/dated a week ago:GCC Compiler Induced Vulnerability - affects programs compiled with GCC 7 and 8 containing nested functions
From: Andrew Sandoval
Date: Mon, 22 Oct 2018 15:07:55 +0000
Actually, that was one month and a week ago. :) Also @dscho as it pertains to embargo, the suggestion from those on the distros list was that this should not be embargoed, since 'the only immediately actionable item' was to have "the maintainers of these programs, or distros/contributors", "look into removing the use of nested functions."
For those working on Git for Windows in particular, I highly recommend that this action be taken. It is a relatively simple change, and it will protect Git for Windows (bash, msys2, MinTTY, etc.) from being flagged as malware, and from leaving open a potential vulnerability in those components.
Frankly, I don't think the GCC devs will do anything with it -- though hopefully that is abnormal pessimism on my part. They've left this alone for a very long time, knowing the security risk (which FWIW, is FAR WORSE on Linux). In all likelihood, nobody on Linux is going to use nested C functions, because the OS and etc. offer few APIs that utilize callbacks. And while the opposite is true on Windows, the GCC community at large has historically not found it in their own interest to promote security on Windows. That leaves it up to those who produce Open Source tools for Windows, compiled by GCC to really understand the risk they take when using this half-baked feature of GCC.
Switching to a non-nested C callback will be trivial in most cases, and there is always the option of compiling Windows specific files using g++ with lambda's, which though I have not tested it, I suspect would not suffer the same problem (or the vulnerability would be more widespread than it appears to be.)
-Andrew
alsliahona
on 29 Nov 2018
So where are we using nested functions? I am not aware of this in Git's own source code. It could be in Cygwin. It could be in the MSYS2 runtime. It could be in Bash.
What makes me super dubious about this entire thing is that the report specifically mentions the Git for Windows installer, which is not even compiled using GCC, but it is compiled by InnoSetup, which in turn is a Delphi program. There is literally no GNU software involved there (unless you count the Bash interpreter to drive the release.sh script that orchestrates InnoSetup, and that Bash interpreter certainly won't compile nested functions).
dscho
on 29 Nov 2018
@dscho I don't remember all of the locations, but I can tell you a few places in things used by Git for Windows:
- MinTTY.exe : winctrls.c in doctl() has nested enumwin() on line 110 and used in a call to EnumChildWindows() on line 116
- MinTTY.exe: winput.c in add_switcher() has nested wnd_enum_tabs() on line 165 and used in a call to EnumWindows on line 250
- MinTTY.exe: winmain.c in refresh_tab_titles() has something almost identical to the above, with nested wnd_enum_tabs() on line 418 and call to EnumWindows on line 474. The function update_tab_titles() has the same thing on lines 488 and 508, though interestingly enough, the function win_switch() does something very similar but does NOT nest the callback function. Still, win_sync_tabs() does use a nested function in the call to EnumWindows, and get_my_monitor_info() uses a nested function on line 900 in a call to EnumDisplayMonitors on line 909. The function search_monitors() has 4 nested functions!
You can pretty much find these by searching the Windows specific code for "Enum".
While git.exe may not use any nested functions directly, the supporting binaries certainly do. MinTTY and bash were the two that triggered our protections designed to prevent stack exploitation. Both of these IIRC, are launched during Git for Windows Install.
I hope that helps. Maybe the discussion needs to be moved to bug reports in mintty, bash, etc., but that will not stop Git for Windows from getting the blame when it is detected as making the Stack Executable while installing or starting Git Bash -- unless of course its dependent components are fixed. (Who knows, it is a development community that uses Git for Windows, maybe they will rightly blame the other components.) I can only tell you that my plate is currently too full to identify each offending component and open bugs.
-Andrew Sandoval
alsliahona
on 29 Nov 2018
down the thread, in https://seclists.org/oss-sec/2018/q4/86, it suggests:
From: Yann Droneaud
Date: Tue, 23 Oct 2018 11:41:13 +0200
Hi,
Use -Werror=trampoline to prevent GCC from generating code that require
executable stack:
https://gcc.gnu.org/onlinedocs/gcc-8.2.0/gcc/Warning-Options.html#index-Wtrampolines
It's a recommanded warning from OWASP:
https://www.owasp.org/index.php/C-Based_Toolchain_Hardening#GCC.2FBinutils
==
So maybe that warning error parameter is what needs to be tried during the compile of each of the different parts.
What's not immediately clear to me is how the problem is propagated between these different 'apps' - I'd expect the various git.exe (command line git) to be independent of the bash and of the MSYS2 binaries and not affect each other. Do the different binaries signal to each other for this particular gcc effect?
PhilipOakley
on 29 Nov 2018
* MinTTY.exe : winctrls.c in doctl() has nested enumwin() on line 110 and used in a call to EnumChildWindows() on line 116 * MinTTY.exe: winput.c in add_switcher() has nested wnd_enum_tabs() on line 165 and used in a call to EnumWindows on line 250 * MinTTY.exe: winmain.c in refresh_tab_titles() has something almost identical to the above, with nested wnd_enum_tabs() on line 418 and call to EnumWindows on line 474. The function update_tab_titles() has the same thing on lines 488 and 508, though interestingly enough, the function win_switch() does something very similar but does NOT nest the callback function. Still, win_sync_tabs() does use a nested function in the call to EnumWindows, and get_my_monitor_info() uses a nested function on line 900 in a call to EnumDisplayMonitors on line 909. The function search_monitors() has 4 nested functions!You can pretty much find these by searching the Windows specific code for "Enum".
@mintty could you have a look? If you're tied up at the moment, I can try to find time to take a crack at it.
While git.exe may not use any nested functions directly, the supporting binaries certainly do.
I assumed as much. And probably the Bash (which we run as part of the installer) is the thing that triggered the "Git for Windows installer" to be put on that list.
@alsliahona thank you for all your help. I hope that I can "retribute" by doing my share to see those issues addressed.
@PhilipOakley thank you for the info! And no, I don't think that git.exe is affected per se (I just compiled Git for Windows with -Werror=trampolines and it did not trigger a build error). But git.exe does spawn Bash, and it is commonly called from Git Bash (which opens a MinTTY). So while git.exe itself is okay, Git for Windows is not.
dscho
on 30 Nov 2018
I can try to find time to take a crack at it.
Nevermind, I had a quick look, and while it is a tedious conversion, it is a straight-forward one.
dscho
on 30 Nov 2018
I've just become aware of this issue and yes, mintty does use inner or nested functions at some places.
The issue is an unfortunate coincidence of several factors:
- inner functions used in mintty
- gcc implementing inner functions with dynamic code ("trampolines")
- some tool detecting such code and over-interpreting it as threat
I think the main culprit here is actually gcc which insanely selects the trampoline technique for implementation of simple inner functions, which is certainly unnecessary overkill.
Maybe I should open a gcc issue about it.
mintty
on 30 Nov 2018
@mintty I would be surprised if there was no GCC ticket about it already. And I would be surprised if they acted quickly on this ;-)
In any case, I am pretty far along with my changes, and hope that you will look favorably on my upcoming PR.
dscho
on 30 Nov 2018
@mintty could you please have a look at https://github.com/mintty/mintty/pull/828? I know, it is a bit tedious to review, and I am sorry about that. Rest assured that it was quite tedious to work on it, too. But I hope that together, we can get this done.
dscho
on 30 Nov 2018
Next stop: Bash.
dscho
on 30 Nov 2018
Thanks, commenting details there.
For the records, I do not particularly like this workaround as nested functions are a valid design choice in many cases and had been so long before the language C spoiled this concept. But I recognize that this may be a necessary tweak in order to resolve the security tool issues reported here.
mintty
on 30 Nov 2018
Next stop: Bash.
Well, this is disappointing. Bash compiles without error when using -Werror=trampolines!
dscho
on 30 Nov 2018
Bash compiles without error when using
-Werror=trampolines!
... which is unsurprising, given that I could not find a single nested function...
dscho
on 30 Nov 2018
And even the MSYS2 runtime compiles cleanly with -Werror=trampolines. Manual inspection reveals that there are exactly two calls to the Enum*() family of functions that take a function pointer, both in winsup/cygwin/fhandler_console.cc, and neither of these function pointers (enum_windows and enum_proc) refer to nested functions.
That really only leaves MinTTY, which I am optimistic we can handle.
But the report talks specifically about the Git for Windows installer, which does not launch any MinTTY, at least not by default.
So maybe the tests were performed on an old Git for Windows version with a Bash or an MSYS2 runtime that contained trampolined nested functions.
At this point, I am really at a loss what I can do about this ticket, other than the MinTTY PR I put up.
dscho
on 30 Nov 2018
At this point, I am really at a loss what I can do about this ticket, other than the MinTTY PR I put up.
@dscho, I think that is more than sufficient at this stage. You have clarified (via the -Werror=trampolines) which parts of the code actually has the potential problem (i.e. only one!).
I suspect that the original Dell bug report was a consequence of the install/usage cascade, which will (I guess) blame the initiating code, not the component that is used consequentially. I guess the G-f-W code is different from most other Windows code in the way it has the *nix component separations.
If the MYSS2 PR is accepted before v2.20, we should see then whether that was the only true 'issue' for the report.
I'm not sure if the Dell system can be told that this is acceptable in the case or whether @shoelzer has any choice about the package acceptability. It would probably be unwelcome if G-f-W had to carry the PR patches just to pacify the Dell protection system..
PhilipOakley
on 30 Nov 2018
@PhilipOakley Just to clarify, I created this issue in June to report a problem with the Dell protection tool and/or Git Bash. The oss-sec email from October (duplicated and linked above) mentioned "Git for Windows Installer" and that may be a separate report or it may be a misinterpretation of this issue. I'm not sure how the Installer got on that list.
Finally, as I mentioned earlier, my company is no longer using the Dell protection tool so that solved my immediate problem. It also means that I can't test any potential fixes.
shoelzer
on 30 Nov 2018
It also means that I can't test any potential fixes.
Okay. @shoelzer thank you for your report and your feedback! I will close this here ticket for now, as it is done.
dscho
on 30 Nov 2018
Related issues
yegorich
·
3Comments
JoshSchreuder
·
4Comments
daxelrod
·
4Comments
vocaviking
·
5Comments
dlk-pavan
·
4Comments