Hesham/wintun
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

api: bail out on _TRUNCATE truncation

Browse Source 
Silently ignoring truncation of the strings(like adapter and pool names,
registry paths etc.) leads to strange failures later down the road (like
registry key not found) masking the true reason of the failure. This
makes troubleshooting difficult.

Reported-by: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Simon Rozman <simon@rozman.si>
This commit is contained in:
Simon Rozman 2020-10-30 06:51:24 +01:00
parent 13e90b52cc
commit 254a900a76
1 changed files with 91 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
api/adapter.c
View File

@ -431,10 +431,12 @@ RemoveNumberedSuffix(_Inout_z_ WCHAR *Name)
} }
} }
static void static WINTUN_STATUS
GetPoolDeviceTypeName(_In_z_count_c_(MAX_POOL) const WCHAR *Pool, _Out_cap_c_(MAX_POOL_DEVICE_TYPE) WCHAR *Name) GetPoolDeviceTypeName(_In_z_count_c_(MAX_POOL) const WCHAR *Pool, _Out_cap_c_(MAX_POOL_DEVICE_TYPE) WCHAR *Name)
{ {
_snwprintf_s(Name, MAX_POOL_DEVICE_TYPE, _TRUNCATE, L"%.*s Tunnel", MAX_POOL, Pool); if (_snwprintf_s(Name, MAX_POOL_DEVICE_TYPE, _TRUNCATE, L"%.*s Tunnel", MAX_POOL, Pool) == -1)
return LOG(WINTUN_LOG_ERR, L"Pool name too long"), ERROR_INVALID_PARAMETER;
return ERROR_SUCCESS;
} }
static WINTUN_STATUS static WINTUN_STATUS
@ -458,7 +460,9 @@ IsPoolMember(
goto cleanupDeviceDesc; goto cleanupDeviceDesc;
} }
WCHAR PoolDeviceTypeName[MAX_POOL_DEVICE_TYPE]; WCHAR PoolDeviceTypeName[MAX_POOL_DEVICE_TYPE];
GetPoolDeviceTypeName(Pool, PoolDeviceTypeName); Result = GetPoolDeviceTypeName(Pool, PoolDeviceTypeName);
if (Result != ERROR_SUCCESS)
goto cleanupFriendlyName;
if (!_wcsicmp(FriendlyName, PoolDeviceTypeName) || !_wcsicmp(DeviceDesc, PoolDeviceTypeName)) if (!_wcsicmp(FriendlyName, PoolDeviceTypeName) || !_wcsicmp(DeviceDesc, PoolDeviceTypeName))
{ {
*IsMember = TRUE; *IsMember = TRUE;
@ -542,7 +546,12 @@ CreateAdapterData(
goto cleanupAdapter; goto cleanupAdapter;
} }
wcsncpy_s((*Adapter)->Pool, _countof((*Adapter)->Pool), Pool, _TRUNCATE); if (wcsncpy_s((*Adapter)->Pool, _countof((*Adapter)->Pool), Pool, _TRUNCATE) == STRUNCATE)
{
LOG(WINTUN_LOG_ERR, L"Pool name too long");
Result = ERROR_INVALID_PARAMETER;
goto cleanupAdapter;
}
Result = ERROR_SUCCESS; Result = ERROR_SUCCESS;
cleanupAdapter: cleanupAdapter:
@ -553,16 +562,18 @@ cleanupKey:
return Result; return Result;
} }
static void static WINTUN_STATUS
GetDeviceRegPath(_In_ const WINTUN_ADAPTER *Adapter, _Out_cap_c_(MAX_REG_PATH) WCHAR *Path) GetDeviceRegPath(_In_ const WINTUN_ADAPTER *Adapter, _Out_cap_c_(MAX_REG_PATH) WCHAR *Path)
{ {
_snwprintf_s( if (_snwprintf_s(
Path, Path,
MAX_REG_PATH, MAX_REG_PATH,
_TRUNCATE, _TRUNCATE,
L"SYSTEM\\CurrentControlSet\\Enum\\%.*s", L"SYSTEM\\CurrentControlSet\\Enum\\%.*s",
MAX_INSTANCE_ID, MAX_INSTANCE_ID,
Adapter->DevInstanceID); Adapter->DevInstanceID) == -1)
return LOG(WINTUN_LOG_ERR, L"Registry path too long"), ERROR_INVALID_PARAMETER;
return ERROR_SUCCESS;
} }
void WINAPI void WINAPI
@ -680,7 +691,8 @@ WintunSetAdapterName(_In_ const WINTUN_ADAPTER *Adapter, _In_z_count_c_(MAX_ADAP
DWORD Result; DWORD Result;
const int MaxSuffix = 1000; const int MaxSuffix = 1000;
WCHAR AvailableName[MAX_ADAPTER_NAME]; WCHAR AvailableName[MAX_ADAPTER_NAME];
wcsncpy_s(AvailableName, _countof(AvailableName), Name, _TRUNCATE); if (wcsncpy_s(AvailableName, _countof(AvailableName), Name, _TRUNCATE) == STRUNCATE)
return LOG(WINTUN_LOG_ERR, L"Pool name too long"), ERROR_INVALID_PARAMETER;
for (int i = 0;; ++i) for (int i = 0;; ++i)
{ {
Result = NciSetConnectionName(&Adapter->CfgInstanceID, AvailableName); Result = NciSetConnectionName(&Adapter->CfgInstanceID, AvailableName);
@ -693,7 +705,9 @@ WintunSetAdapterName(_In_ const WINTUN_ADAPTER *Adapter, _In_z_count_c_(MAX_ADAP
for (int j = 0; j < MaxSuffix; ++j) for (int j = 0; j < MaxSuffix; ++j)
{ {
WCHAR Proposal[MAX_ADAPTER_NAME]; WCHAR Proposal[MAX_ADAPTER_NAME];
_snwprintf_s(Proposal, _countof(Proposal), _TRUNCATE, L"%.*s %d", MAX_ADAPTER_NAME, Name, j + 1); if (_snwprintf_s(
Proposal, _countof(Proposal), _TRUNCATE, L"%.*s %d", MAX_ADAPTER_NAME, Name, j + 1) == -1)
return LOG(WINTUN_LOG_ERR, L"Pool name too long"), ERROR_INVALID_PARAMETER;
if (_wcsnicmp(Proposal, AvailableName, MAX_ADAPTER_NAME) == 0) if (_wcsnicmp(Proposal, AvailableName, MAX_ADAPTER_NAME) == 0)
continue; continue;
Result2 = NciSetConnectionName(&Guid2, Proposal); Result2 = NciSetConnectionName(&Guid2, Proposal);
@ -713,18 +727,24 @@ WintunSetAdapterName(_In_ const WINTUN_ADAPTER *Adapter, _In_z_count_c_(MAX_ADAP
break; break;
if (i > MaxSuffix || Result != ERROR_DUP_NAME) if (i > MaxSuffix || Result != ERROR_DUP_NAME)
return LOG_ERROR(L"Setting adapter name failed", Result); return LOG_ERROR(L"Setting adapter name failed", Result);
_snwprintf_s(AvailableName, _countof(AvailableName), _TRUNCATE, L"%.*s %d", MAX_ADAPTER_NAME, Name, i + 1); if (_snwprintf_s(
AvailableName, _countof(AvailableName), _TRUNCATE, L"%.*s %d", MAX_ADAPTER_NAME, Name, i + 1) == -1)
return LOG(WINTUN_LOG_ERR, L"Pool name too long"), ERROR_INVALID_PARAMETER;
} }
/* TODO: This should use NetSetup2 so that it doesn't get unset. */ /* TODO: This should use NetSetup2 so that it doesn't get unset. */
HKEY DeviceRegKey; HKEY DeviceRegKey;
WCHAR DeviceRegPath[MAX_REG_PATH]; WCHAR DeviceRegPath[MAX_REG_PATH];
GetDeviceRegPath(Adapter, DeviceRegPath); Result = GetDeviceRegPath(Adapter, DeviceRegPath);
if (Result != ERROR_SUCCESS)
return Result;
Result = RegOpenKeyExW(HKEY_LOCAL_MACHINE, DeviceRegPath, 0, KEY_SET_VALUE, &DeviceRegKey); Result = RegOpenKeyExW(HKEY_LOCAL_MACHINE, DeviceRegPath, 0, KEY_SET_VALUE, &DeviceRegKey);
if (Result != ERROR_SUCCESS) if (Result != ERROR_SUCCESS)
return LOG_ERROR(L"Failed to open registry key", Result); return LOG_ERROR(L"Failed to open registry key", Result);
WCHAR PoolDeviceTypeName[MAX_POOL_DEVICE_TYPE]; WCHAR PoolDeviceTypeName[MAX_POOL_DEVICE_TYPE];
GetPoolDeviceTypeName(Adapter->Pool, PoolDeviceTypeName); Result = GetPoolDeviceTypeName(Adapter->Pool, PoolDeviceTypeName);
if (Result != ERROR_SUCCESS)
goto cleanupDeviceRegKey;
Result = RegSetKeyValueW( Result = RegSetKeyValueW(
DeviceRegKey, DeviceRegKey,
NULL, NULL,
@ -732,6 +752,7 @@ WintunSetAdapterName(_In_ const WINTUN_ADAPTER *Adapter, _In_z_count_c_(MAX_ADAP
REG_SZ, REG_SZ,
PoolDeviceTypeName, PoolDeviceTypeName,
(DWORD)((wcslen(PoolDeviceTypeName) + 1) * sizeof(WCHAR))); (DWORD)((wcslen(PoolDeviceTypeName) + 1) * sizeof(WCHAR)));
cleanupDeviceRegKey:
RegCloseKey(DeviceRegKey); RegCloseKey(DeviceRegKey);
return Result; return Result;
} }
@ -875,17 +896,19 @@ IsNewer(_In_ const SP_DRVINFO_DATA_W *DrvInfoData, _In_ const FILETIME *DriverDa
return FALSE; return FALSE;
} }
static void static DWORD
GetTcpipAdapterRegPath(_In_ const WINTUN_ADAPTER *Adapter, _Out_cap_c_(MAX_REG_PATH) WCHAR *Path) GetTcpipAdapterRegPath(_In_ const WINTUN_ADAPTER *Adapter, _Out_cap_c_(MAX_REG_PATH) WCHAR *Path)
{ {
WCHAR Guid[MAX_GUID_STRING_LEN]; WCHAR Guid[MAX_GUID_STRING_LEN];
_snwprintf_s( if (_snwprintf_s(
Path, Path,
MAX_REG_PATH, MAX_REG_PATH,
_TRUNCATE, _TRUNCATE,
L"SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters\\Adapters\\%.*s", L"SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters\\Adapters\\%.*s",
StringFromGUID2(&Adapter->CfgInstanceID, Guid, _countof(Guid)), StringFromGUID2(&Adapter->CfgInstanceID, Guid, _countof(Guid)),
Guid); Guid) == -1)
return LOG(WINTUN_LOG_ERR, L"Registry path too long"), ERROR_INVALID_PARAMETER;
return ERROR_SUCCESS;
} }
static WINTUN_STATUS static WINTUN_STATUS
@ -894,7 +917,9 @@ GetTcpipInterfaceRegPath(_In_ const WINTUN_ADAPTER *Adapter, _Out_cap_c_(MAX_REG
DWORD Result; DWORD Result;
HKEY TcpipAdapterRegKey; HKEY TcpipAdapterRegKey;
WCHAR TcpipAdapterRegPath[MAX_REG_PATH]; WCHAR TcpipAdapterRegPath[MAX_REG_PATH];
GetTcpipAdapterRegPath(Adapter, TcpipAdapterRegPath); Result = GetTcpipAdapterRegPath(Adapter, TcpipAdapterRegPath);
if (Result != ERROR_SUCCESS)
return Result;
Result = RegOpenKeyExW(HKEY_LOCAL_MACHINE, TcpipAdapterRegPath, 0, KEY_QUERY_VALUE, &TcpipAdapterRegKey); Result = RegOpenKeyExW(HKEY_LOCAL_MACHINE, TcpipAdapterRegPath, 0, KEY_QUERY_VALUE, &TcpipAdapterRegKey);
if (Result != ERROR_SUCCESS) if (Result != ERROR_SUCCESS)
return LOG_ERROR(L"Failed to open registry key", Result); return LOG_ERROR(L"Failed to open registry key", Result);
@ -911,7 +936,13 @@ GetTcpipInterfaceRegPath(_In_ const WINTUN_ADAPTER *Adapter, _Out_cap_c_(MAX_REG
Result = ERROR_INVALID_DATA; Result = ERROR_INVALID_DATA;
goto cleanupPaths; goto cleanupPaths;
} }
_snwprintf_s(Path, MAX_REG_PATH, _TRUNCATE, L"SYSTEM\\CurrentControlSet\\Services\\%s", Paths); if (_snwprintf_s(Path, MAX_REG_PATH, _TRUNCATE, L"SYSTEM\\CurrentControlSet\\Services\\%s", Paths) == -1)
{
LOG(WINTUN_LOG_ERR, L"Registry path too long");
Result = ERROR_INVALID_PARAMETER;
goto cleanupPaths;
}
Result = ERROR_SUCCESS;
cleanupPaths: cleanupPaths:
HeapFree(ModuleHeap, 0, Paths); HeapFree(ModuleHeap, 0, Paths);
cleanupTcpipAdapterRegKey: cleanupTcpipAdapterRegKey:
@ -948,7 +979,9 @@ CreateAdapter(
} }
WCHAR PoolDeviceTypeName[MAX_POOL_DEVICE_TYPE]; WCHAR PoolDeviceTypeName[MAX_POOL_DEVICE_TYPE];
GetPoolDeviceTypeName(Pool, PoolDeviceTypeName); Result = GetPoolDeviceTypeName(Pool, PoolDeviceTypeName);
if (Result != ERROR_SUCCESS)
goto cleanupDevInfo;
SP_DEVINFO_DATA DevInfoData = { .cbSize = sizeof(SP_DEVINFO_DATA) }; SP_DEVINFO_DATA DevInfoData = { .cbSize = sizeof(SP_DEVINFO_DATA) };
if (!SetupDiCreateDeviceInfoW( if (!SetupDiCreateDeviceInfoW(
DevInfo, ClassName, &GUID_DEVCLASS_NET, PoolDeviceTypeName, NULL, DICD_GENERATE_ID, &DevInfoData)) DevInfo, ClassName, &GUID_DEVCLASS_NET, PoolDeviceTypeName, NULL, DICD_GENERATE_ID, &DevInfoData))
@ -1125,7 +1158,9 @@ CreateAdapter(
HKEY TcpipAdapterRegKey; HKEY TcpipAdapterRegKey;
WCHAR TcpipAdapterRegPath[MAX_REG_PATH]; WCHAR TcpipAdapterRegPath[MAX_REG_PATH];
GetTcpipAdapterRegPath(*Adapter, TcpipAdapterRegPath); Result = GetTcpipAdapterRegPath(*Adapter, TcpipAdapterRegPath);
if (Result != ERROR_SUCCESS)
goto cleanupAdapter;
Result = RegistryOpenKeyWait( Result = RegistryOpenKeyWait(
HKEY_LOCAL_MACHINE, HKEY_LOCAL_MACHINE,
TcpipAdapterRegPath, TcpipAdapterRegPath,
@ -1343,7 +1378,13 @@ ExecuteRunDll32(
Result = ERROR_OUTOFMEMORY; Result = ERROR_OUTOFMEMORY;
goto cleanupDelete; goto cleanupDelete;
} }
_snwprintf_s(CommandLine, CommandLineLen, _TRUNCATE, L"rundll32 \"%.*s\",%s", MAX_PATH, DllPath, Arguments); if (_snwprintf_s(CommandLine, CommandLineLen, _TRUNCATE, L"rundll32 \"%.*s\",%s", MAX_PATH, DllPath, Arguments) ==
-1)
{
LOG(WINTUN_LOG_ERR, L"Command line too long");
Result = ERROR_INVALID_PARAMETER;
goto cleanupDelete;
}
SECURITY_ATTRIBUTES sa = { .nLength = sizeof(SECURITY_ATTRIBUTES), SECURITY_ATTRIBUTES sa = { .nLength = sizeof(SECURITY_ATTRIBUTES),
.bInheritHandle = TRUE, .bInheritHandle = TRUE,
.lpSecurityDescriptor = .lpSecurityDescriptor =
@ -1454,7 +1495,7 @@ WintunCreateAdapter(
LOG(WINTUN_LOG_INFO, L"Spawning native process for the job"); LOG(WINTUN_LOG_INFO, L"Spawning native process for the job");
WCHAR RequestedGUIDStr[MAX_GUID_STRING_LEN]; WCHAR RequestedGUIDStr[MAX_GUID_STRING_LEN];
WCHAR Arguments[15 + MAX_POOL + 3 + MAX_ADAPTER_NAME + 2 + MAX_GUID_STRING_LEN + 1]; WCHAR Arguments[15 + MAX_POOL + 3 + MAX_ADAPTER_NAME + 2 + MAX_GUID_STRING_LEN + 1];
_snwprintf_s( if (_snwprintf_s(
Arguments, Arguments,
_countof(Arguments), _countof(Arguments),
_TRUNCATE, _TRUNCATE,
@ -1464,7 +1505,8 @@ WintunCreateAdapter(
MAX_ADAPTER_NAME, MAX_ADAPTER_NAME,
Name, Name,
RequestedGUID ? StringFromGUID2(RequestedGUID, RequestedGUIDStr, _countof(RequestedGUIDStr)) : 0, RequestedGUID ? StringFromGUID2(RequestedGUID, RequestedGUIDStr, _countof(RequestedGUIDStr)) : 0,
RequestedGUIDStr); RequestedGUIDStr) == -1)
return LOG(WINTUN_LOG_ERR, L"Command line too long"), ERROR_INVALID_PARAMETER;
WCHAR Response[8 + 1 + MAX_GUID_STRING_LEN + 1 + 8 + 1]; WCHAR Response[8 + 1 + MAX_GUID_STRING_LEN + 1 + 8 + 1];
DWORD Result = ExecuteRunDll32(Arguments, Response, _countof(Response)); DWORD Result = ExecuteRunDll32(Arguments, Response, _countof(Response));
if (Result != ERROR_SUCCESS) if (Result != ERROR_SUCCESS)
@ -1555,13 +1597,14 @@ WintunDeleteAdapter(_In_ const WINTUN_ADAPTER *Adapter, _Inout_ BOOL *RebootRequ
LOG(WINTUN_LOG_INFO, L"Spawning native process for the job"); LOG(WINTUN_LOG_INFO, L"Spawning native process for the job");
WCHAR GuidStr[MAX_GUID_STRING_LEN]; WCHAR GuidStr[MAX_GUID_STRING_LEN];
WCHAR Arguments[14 + MAX_GUID_STRING_LEN + 1]; WCHAR Arguments[14 + MAX_GUID_STRING_LEN + 1];
_snwprintf_s( if (_snwprintf_s(
Arguments, Arguments,
_countof(Arguments), _countof(Arguments),
_TRUNCATE, _TRUNCATE,
L"DeleteAdapter %.*s", L"DeleteAdapter %.*s",
StringFromGUID2(&Adapter->CfgInstanceID, GuidStr, _countof(GuidStr)), StringFromGUID2(&Adapter->CfgInstanceID, GuidStr, _countof(GuidStr)),
GuidStr); GuidStr) == -1)
return LOG(WINTUN_LOG_ERR, L"Command line too long"), ERROR_INVALID_PARAMETER;
WCHAR Response[8 + 1 + 8 + 1]; WCHAR Response[8 + 1 + 8 + 1];
DWORD Result = ExecuteRunDll32(Arguments, Response, _countof(Response)); DWORD Result = ExecuteRunDll32(Arguments, Response, _countof(Response));
if (Result != ERROR_SUCCESS) if (Result != ERROR_SUCCESS)