Procházet zdrojové kódy

some fixes, make sure the failfast path not compromised

Lei Shi před 9 roky
rodič
revize
aebdefe204

+ 1 - 4
lib/Backend/CodeGenNumberAllocator.cpp

@@ -329,9 +329,7 @@ Js::JavascriptNumber* XProcNumberPageSegmentImpl::AllocateNumber(Func* func, dou
             *(void**)pLocalNumber = (void*)func->GetScriptContextInfo()->GetVTableAddress(VTableValue::VtableJavascriptNumber);
 
             // initialize number by WriteProcessMemory
-            SIZE_T bytesWritten;
-            if (!WriteProcessMemory(hProcess, (void*)number, pLocalNumber, sizeCat, &bytesWritten)
-                || bytesWritten != sizeCat)
+            if (!WriteProcessMemory(hProcess, (void*)number, pLocalNumber, sizeCat, NULL))
             {
                 MemoryOperationLastError::RecordLastError();
                 Js::Throw::InternalError();
@@ -377,7 +375,6 @@ Js::JavascriptNumber* XProcNumberPageSegmentImpl::AllocateNumber(Func* func, dou
         XProcNumberPageSegmentImpl* seg = (XProcNumberPageSegmentImpl*)midl_user_allocate(sizeof(XProcNumberPageSegment));
         if (seg == nullptr)
         {
-            MemoryOperationLastError::RecordLastError();
             Js::Throw::OutOfMemory();
         }
         seg = new (seg) XProcNumberPageSegmentImpl();

+ 12 - 18
lib/Common/Memory/PageAllocator.cpp

@@ -218,11 +218,8 @@ PageSegmentBase<T>::Initialize(DWORD allocFlags, bool excludeGuardPages)
             BOOL vpresult = VirtualProtectEx(this->GetAllocator()->processHandle, this->address, this->GetAvailablePageCount() * AutoSystemInfo::PageSize, PAGE_NOACCESS, &oldProtect);
             if (vpresult == FALSE)
             {
+                MemoryOperationLastError::RecordLastError();
                 if (this->allocator->processHandle == GetCurrentProcess())
-                {
-                    MemoryOperationLastError::RecordLastError();
-                }
-                else
                 {
                     Assert(false);
                 }
@@ -314,13 +311,10 @@ PageSegmentBase<T>::AllocPages(uint pageCount)
                 BOOL vpresult = VirtualProtectEx(this->GetAllocator()->processHandle, allocAddress, pageCount * AutoSystemInfo::PageSize, PAGE_READWRITE, &oldProtect);
             if (vpresult == FALSE)
             {
+                MemoryOperationLastError::RecordLastError();
                 if (this->GetAllocator()->processHandle == GetCurrentProcess())
                 {
-                    MemoryOperationLastError::RecordLastError();
-                }
-                else
-                {
-                    Assert(false);
+                    Assert(false);                    
                 }
                 return nullptr;
             }
@@ -426,11 +420,8 @@ PageSegmentBase<T>::ReleasePages(__in void * address, uint pageCount)
     BOOL vpresult = VirtualProtectEx(this->GetAllocator()->processHandle, address, pageCount * AutoSystemInfo::PageSize, PAGE_NOACCESS, &oldProtect);
     if (vpresult == FALSE)
     {
+        MemoryOperationLastError::RecordLastError();
         if (this->allocator->processHandle == GetCurrentProcess())
-        {
-            MemoryOperationLastError::RecordLastError();
-        }
-        else
         {
             Assert(false);
         }
@@ -2448,15 +2439,18 @@ HeapPageAllocator<T>::ProtectPages(__in char* address, size_t pageCount, __in vo
 
     // check old protection on all pages about to change, ensure the fidelity
     size_t bytes = VirtualQueryEx(this->processHandle, address, &memBasicInfo, sizeof(memBasicInfo));
+    if (bytes == 0)
+    {
+        MemoryOperationLastError::RecordLastError();
+    }
     if (bytes == 0
         || memBasicInfo.RegionSize < pageCount * AutoSystemInfo::PageSize
         || desiredOldProtectFlag != memBasicInfo.Protect)
     {
-        if (this->processHandle == GetCurrentProcess())
-        {
-            MemoryOperationLastError::RecordLastError();
-        }
-        else
+#if ENABLE_OOP_NATIVE_CODEGEN
+        if (this->processHandle == GetCurrentProcess()
+            || GetProcessId(this->processHandle) == GetCurrentProcessId()) // in case processHandle is modified and exploited(duplicated current process handle)
+#endif
         {
             CustomHeap_BadPageState_fatal_error((ULONG_PTR)this);
         }

+ 25 - 17
lib/Common/Memory/VirtualAllocWrapper.cpp

@@ -58,7 +58,10 @@ LPVOID VirtualAllocWrapper::Alloc(LPVOID lpAddress, size_t dwSize, DWORD allocat
             if (result == FALSE)
             {
                 MemoryOperationLastError::RecordLastError();
-                if (process == GetCurrentProcess())
+#if ENABLE_OOP_NATIVE_CODEGEN
+                if (process == GetCurrentProcess()
+                    || GetProcessId(process) == GetCurrentProcessId()) // in case processHandle is modified and exploited(duplicated current process handle)
+#endif
                 {
                     CustomHeap_BadPageState_fatal_error((ULONG_PTR)this);
                 }
@@ -143,15 +146,12 @@ PreReservedVirtualAllocWrapper::IsInRange(void * address)
     size_t bytes = VirtualQueryEx(processHandle, address, &memBasicInfo, sizeof(memBasicInfo));
     if (bytes == 0)
     {
+        MemoryOperationLastError::RecordLastError();
         if (this->processHandle != GetCurrentProcess())
         {
-            MemoryOperationLastError::RecordLastError();
-            if (this->processHandle != GetCurrentProcess())
-            {
-                Js::Throw::InternalError();
-            }
-            return false;
+            Js::Throw::InternalError();
         }
+        return false;
     }
     AssertMsg(memBasicInfo.State == MEM_COMMIT, "Memory not committed? Checking for uncommitted address region?");
 #endif
@@ -320,15 +320,18 @@ LPVOID PreReservedVirtualAllocWrapper::Alloc(LPVOID lpAddress, size_t dwSize, DW
             //Check if the region is not already in MEM_COMMIT state.
             MEMORY_BASIC_INFORMATION memBasicInfo;
             size_t bytes = VirtualQueryEx(processHandle, addressToReserve, &memBasicInfo, sizeof(memBasicInfo));
+            if (bytes == 0) 
+            {
+                MemoryOperationLastError::RecordLastError();
+            }
             if (bytes == 0
                 || memBasicInfo.RegionSize < requestedNumOfSegments * AutoSystemInfo::Data.GetAllocationGranularityPageSize()
                 || memBasicInfo.State == MEM_COMMIT)
             {
-                if (this->processHandle != GetCurrentProcess())
-                {
-                    MemoryOperationLastError::RecordLastError();
-                }
-                else
+#if ENABLE_OOP_NATIVE_CODEGEN
+                if (this->processHandle == GetCurrentProcess()
+                    || GetProcessId(this->processHandle) == GetCurrentProcessId()) // in case processHandle is modified and exploited(duplicated current process handle)
+#endif
                 {
                     CustomHeap_BadPageState_fatal_error((ULONG_PTR)this);
                 }
@@ -378,15 +381,20 @@ LPVOID PreReservedVirtualAllocWrapper::Alloc(LPVOID lpAddress, size_t dwSize, DW
                 }
 
                 allocatedAddress = (char *)VirtualAllocEx(processHandle, addressToReserve, dwSize, MEM_COMMIT, allocProtectFlags);
-
                 if (allocatedAddress != nullptr)
                 {
                     BOOL result = VirtualProtectEx(processHandle, allocatedAddress, dwSize, protectFlags, &oldProtect);
-                    if (result == FALSE && this->processHandle != GetCurrentProcess())
+                    if (result == FALSE)
                     {
-                        // TODO: need to free the page?
-                        MemoryOperationLastError::RecordLastError();
                         failedToProtectPages = true;
+                        MemoryOperationLastError::RecordLastError();
+#if ENABLE_OOP_NATIVE_CODEGEN
+                        if (this->processHandle == GetCurrentProcess()
+                            || GetProcessId(this->processHandle) == GetCurrentProcessId())
+#endif
+                        {
+                            CustomHeap_BadPageState_fatal_error((ULONG_PTR)this);
+                        }
                     }
                     AssertMsg(oldProtect == (PAGE_EXECUTE_READWRITE), "CFG Bitmap gets allocated and bits will be set to invalid only upon passing these flags.");
                 }
@@ -399,7 +407,7 @@ LPVOID PreReservedVirtualAllocWrapper::Alloc(LPVOID lpAddress, size_t dwSize, DW
 #endif
             {
                 allocatedAddress = (char *)VirtualAllocEx(processHandle, addressToReserve, dwSize, MEM_COMMIT, protectFlags);
-                if (allocatedAddress == nullptr && this->processHandle != GetCurrentProcess())
+                if (allocatedAddress == nullptr)
                 {
                     MemoryOperationLastError::RecordLastError();
                 }