Commit 6f396e25 authored by cpu's avatar cpu Committed by Commit bot

fix sandbox memory leak

The memory allocated by AllocAndCopyName was not being freed
if the in-process policy engine did not allow the request to
query the broker.

This was nicely reported by typo.pl@gmail.com

TEST=see bug
BUG=414039

Review URL: https://codereview.chromium.org/575623004

Cr-Commit-Position: refs/heads/master@{#295220}
parent 7a2b1cb8
...@@ -35,6 +35,7 @@ NTSTATUS WINAPI TargetNtCreateFile(NtCreateFileFunction orig_CreateFile, ...@@ -35,6 +35,7 @@ NTSTATUS WINAPI TargetNtCreateFile(NtCreateFileFunction orig_CreateFile,
if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled())
return status; return status;
wchar_t* name = NULL;
do { do {
if (!ValidParameter(file, sizeof(HANDLE), WRITE)) if (!ValidParameter(file, sizeof(HANDLE), WRITE))
break; break;
...@@ -45,7 +46,6 @@ NTSTATUS WINAPI TargetNtCreateFile(NtCreateFileFunction orig_CreateFile, ...@@ -45,7 +46,6 @@ NTSTATUS WINAPI TargetNtCreateFile(NtCreateFileFunction orig_CreateFile,
if (NULL == memory) if (NULL == memory)
break; break;
wchar_t* name;
uint32 attributes = 0; uint32 attributes = 0;
NTSTATUS ret = AllocAndCopyName(object_attributes, &name, &attributes, NTSTATUS ret = AllocAndCopyName(object_attributes, &name, &attributes,
NULL); NULL);
...@@ -69,9 +69,6 @@ NTSTATUS WINAPI TargetNtCreateFile(NtCreateFileFunction orig_CreateFile, ...@@ -69,9 +69,6 @@ NTSTATUS WINAPI TargetNtCreateFile(NtCreateFileFunction orig_CreateFile,
ResultCode code = CrossCall(ipc, IPC_NTCREATEFILE_TAG, name, attributes, ResultCode code = CrossCall(ipc, IPC_NTCREATEFILE_TAG, name, attributes,
desired_access, file_attributes, sharing, desired_access, file_attributes, sharing,
disposition, options, &answer); disposition, options, &answer);
operator delete(name, NT_ALLOC);
if (SBOX_ALL_OK != code) if (SBOX_ALL_OK != code)
break; break;
...@@ -88,6 +85,9 @@ NTSTATUS WINAPI TargetNtCreateFile(NtCreateFileFunction orig_CreateFile, ...@@ -88,6 +85,9 @@ NTSTATUS WINAPI TargetNtCreateFile(NtCreateFileFunction orig_CreateFile,
} }
} while (false); } while (false);
if (name)
operator delete(name, NT_ALLOC);
return status; return status;
} }
...@@ -106,6 +106,7 @@ NTSTATUS WINAPI TargetNtOpenFile(NtOpenFileFunction orig_OpenFile, PHANDLE file, ...@@ -106,6 +106,7 @@ NTSTATUS WINAPI TargetNtOpenFile(NtOpenFileFunction orig_OpenFile, PHANDLE file,
if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled())
return status; return status;
wchar_t* name = NULL;
do { do {
if (!ValidParameter(file, sizeof(HANDLE), WRITE)) if (!ValidParameter(file, sizeof(HANDLE), WRITE))
break; break;
...@@ -116,7 +117,6 @@ NTSTATUS WINAPI TargetNtOpenFile(NtOpenFileFunction orig_OpenFile, PHANDLE file, ...@@ -116,7 +117,6 @@ NTSTATUS WINAPI TargetNtOpenFile(NtOpenFileFunction orig_OpenFile, PHANDLE file,
if (NULL == memory) if (NULL == memory)
break; break;
wchar_t* name;
uint32 attributes; uint32 attributes;
NTSTATUS ret = AllocAndCopyName(object_attributes, &name, &attributes, NTSTATUS ret = AllocAndCopyName(object_attributes, &name, &attributes,
NULL); NULL);
...@@ -137,9 +137,6 @@ NTSTATUS WINAPI TargetNtOpenFile(NtOpenFileFunction orig_OpenFile, PHANDLE file, ...@@ -137,9 +137,6 @@ NTSTATUS WINAPI TargetNtOpenFile(NtOpenFileFunction orig_OpenFile, PHANDLE file,
CrossCallReturn answer = {0}; CrossCallReturn answer = {0};
ResultCode code = CrossCall(ipc, IPC_NTOPENFILE_TAG, name, attributes, ResultCode code = CrossCall(ipc, IPC_NTOPENFILE_TAG, name, attributes,
desired_access, sharing, options, &answer); desired_access, sharing, options, &answer);
operator delete(name, NT_ALLOC);
if (SBOX_ALL_OK != code) if (SBOX_ALL_OK != code)
break; break;
...@@ -156,6 +153,9 @@ NTSTATUS WINAPI TargetNtOpenFile(NtOpenFileFunction orig_OpenFile, PHANDLE file, ...@@ -156,6 +153,9 @@ NTSTATUS WINAPI TargetNtOpenFile(NtOpenFileFunction orig_OpenFile, PHANDLE file,
} }
} while (false); } while (false);
if (name)
operator delete(name, NT_ALLOC);
return status; return status;
} }
...@@ -172,6 +172,7 @@ NTSTATUS WINAPI TargetNtQueryAttributesFile( ...@@ -172,6 +172,7 @@ NTSTATUS WINAPI TargetNtQueryAttributesFile(
if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled())
return status; return status;
wchar_t* name = NULL;
do { do {
if (!ValidParameter(file_attributes, sizeof(FILE_BASIC_INFORMATION), WRITE)) if (!ValidParameter(file_attributes, sizeof(FILE_BASIC_INFORMATION), WRITE))
break; break;
...@@ -180,7 +181,6 @@ NTSTATUS WINAPI TargetNtQueryAttributesFile( ...@@ -180,7 +181,6 @@ NTSTATUS WINAPI TargetNtQueryAttributesFile(
if (NULL == memory) if (NULL == memory)
break; break;
wchar_t* name = NULL;
uint32 attributes = 0; uint32 attributes = 0;
NTSTATUS ret = AllocAndCopyName(object_attributes, &name, &attributes, NTSTATUS ret = AllocAndCopyName(object_attributes, &name, &attributes,
NULL); NULL);
...@@ -212,6 +212,9 @@ NTSTATUS WINAPI TargetNtQueryAttributesFile( ...@@ -212,6 +212,9 @@ NTSTATUS WINAPI TargetNtQueryAttributesFile(
} while (false); } while (false);
if (name)
operator delete(name, NT_ALLOC);
return status; return status;
} }
...@@ -229,6 +232,7 @@ NTSTATUS WINAPI TargetNtQueryFullAttributesFile( ...@@ -229,6 +232,7 @@ NTSTATUS WINAPI TargetNtQueryFullAttributesFile(
if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled())
return status; return status;
wchar_t* name = NULL;
do { do {
if (!ValidParameter(file_attributes, sizeof(FILE_NETWORK_OPEN_INFORMATION), if (!ValidParameter(file_attributes, sizeof(FILE_NETWORK_OPEN_INFORMATION),
WRITE)) WRITE))
...@@ -238,7 +242,6 @@ NTSTATUS WINAPI TargetNtQueryFullAttributesFile( ...@@ -238,7 +242,6 @@ NTSTATUS WINAPI TargetNtQueryFullAttributesFile(
if (NULL == memory) if (NULL == memory)
break; break;
wchar_t* name = NULL;
uint32 attributes = 0; uint32 attributes = 0;
NTSTATUS ret = AllocAndCopyName(object_attributes, &name, &attributes, NTSTATUS ret = AllocAndCopyName(object_attributes, &name, &attributes,
NULL); NULL);
...@@ -269,6 +272,9 @@ NTSTATUS WINAPI TargetNtQueryFullAttributesFile( ...@@ -269,6 +272,9 @@ NTSTATUS WINAPI TargetNtQueryFullAttributesFile(
return answer.nt_status; return answer.nt_status;
} while (false); } while (false);
if (name)
operator delete(name, NT_ALLOC);
return status; return status;
} }
...@@ -286,6 +292,7 @@ NTSTATUS WINAPI TargetNtSetInformationFile( ...@@ -286,6 +292,7 @@ NTSTATUS WINAPI TargetNtSetInformationFile(
if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled())
return status; return status;
wchar_t* name = NULL;
do { do {
void* memory = GetGlobalIPCMemory(); void* memory = GetGlobalIPCMemory();
if (NULL == memory) if (NULL == memory)
...@@ -315,7 +322,6 @@ NTSTATUS WINAPI TargetNtSetInformationFile( ...@@ -315,7 +322,6 @@ NTSTATUS WINAPI TargetNtSetInformationFile(
break; break;
} }
wchar_t* name;
NTSTATUS ret = AllocAndCopyName(&object_attributes, &name, NULL, NULL); NTSTATUS ret = AllocAndCopyName(&object_attributes, &name, NULL, NULL);
if (!NT_SUCCESS(ret) || !name) if (!NT_SUCCESS(ret) || !name)
break; break;
...@@ -345,6 +351,9 @@ NTSTATUS WINAPI TargetNtSetInformationFile( ...@@ -345,6 +351,9 @@ NTSTATUS WINAPI TargetNtSetInformationFile(
status = answer.nt_status; status = answer.nt_status;
} while (false); } while (false);
if (name)
operator delete(name, NT_ALLOC);
return status; return status;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment