From aa60e561990cd11074a13906b7d2467eaaab8698 Mon Sep 17 00:00:00 2001 From: Timo Kreuzer Date: Wed, 9 Apr 2025 17:17:21 +0300 Subject: [PATCH] [NTOS] Fix MSVC warnings Be strict about string length to prevent overflows. --- ntoskrnl/io/iomgr/deviface.c | 6 +++--- ntoskrnl/io/iomgr/driver.c | 32 +++++++++++++++++++++++++------- ntoskrnl/io/pnpmgr/devaction.c | 25 +++++++++++++++++-------- ntoskrnl/ob/oblink.c | 2 +- 4 files changed, 46 insertions(+), 19 deletions(-) diff --git a/ntoskrnl/io/iomgr/deviface.c b/ntoskrnl/io/iomgr/deviface.c index 5c3391f929b..f7d55ed9796 100644 --- a/ntoskrnl/io/iomgr/deviface.c +++ b/ntoskrnl/io/iomgr/deviface.c @@ -824,13 +824,13 @@ IoGetDeviceInterfaceAlias( DeviceString.MaximumLength = DeviceString.Length; DeviceString.Buffer = Buffer; - /* + /* * Separate symbolic link into 4 parts: * 1) prefix string (\??\ for kernel mode or \\?\ for user mode), * 2) munged path string (like ##?#ACPI#PNP0501#1#{GUID}), * 3) GUID string (the current GUID), * 4) reference string (goes after GUID, starts with '\'). - * + * * We need only reference string. */ Status = IopSeparateSymbolicLink(SymbolicLinkName, @@ -1905,7 +1905,7 @@ IoSetDeviceInterfaceState(IN PUNICODE_STRING SymbolicLinkName, } ASSERT(GuidString.Buffer >= LinkNameNoPrefix.Buffer + 1); - DeviceInstance.Length = (GuidString.Buffer - LinkNameNoPrefix.Buffer - 1) * sizeof(WCHAR); + DeviceInstance.Length = (USHORT)((GuidString.Buffer - LinkNameNoPrefix.Buffer - 1) * sizeof(WCHAR)); if (DeviceInstance.Length == 0) { DPRINT1("No device instance in link name '%wZ'\n", SymbolicLinkName); diff --git a/ntoskrnl/io/iomgr/driver.c b/ntoskrnl/io/iomgr/driver.c index 248140a0bf6..34a6b0ca642 100644 --- a/ntoskrnl/io/iomgr/driver.c +++ b/ntoskrnl/io/iomgr/driver.c @@ -5,7 +5,7 @@ * PURPOSE: Driver Object Management * PROGRAMMERS: Alex Ionescu (alex.ionescu@reactos.org) * Filip Navara (navaraf@reactos.org) - * Hervé Poussineau (hpoussin@reactos.org) + * HervĂ© Poussineau (hpoussin@reactos.org) */ /* INCLUDES *******************************************************************/ @@ -136,13 +136,19 @@ IopGetDriverNames( if (NT_SUCCESS(status)) { /* We've got the ObjectName, use it as the driver name */ - if (kvInfo->Type != REG_SZ || kvInfo->DataLength == 0) + if ((kvInfo->Type != REG_SZ) || + (kvInfo->DataLength < sizeof(UNICODE_NULL)) || + (kvInfo->DataLength > UNICODE_STRING_MAX_BYTES) || + ((kvInfo->DataLength % sizeof(WCHAR)) != 0)) { + DPRINT1("ObjectName invalid (Type = %lu, DataLength = %lu)\n", + kvInfo->Type, + kvInfo->DataLength); ExFreePool(kvInfo); return STATUS_ILL_FORMED_SERVICE_ENTRY; } - driverName.Length = kvInfo->DataLength - sizeof(UNICODE_NULL); + driverName.Length = (USHORT)(kvInfo->DataLength - sizeof(UNICODE_NULL)); driverName.MaximumLength = kvInfo->DataLength; driverName.Buffer = ExAllocatePoolWithTag(NonPagedPool, driverName.MaximumLength, TAG_IO); if (!driverName.Buffer) @@ -963,13 +969,19 @@ IopInitializeBuiltinDriver(IN PLDR_DATA_TABLE_ENTRY BootLdrEntry) { continue; } - if (kvInfo->Type != REG_SZ || kvInfo->DataLength == 0) + if ((kvInfo->Type != REG_SZ) || + (kvInfo->DataLength < sizeof(UNICODE_NULL)) || + (kvInfo->DataLength > UNICODE_STRING_MAX_BYTES) || + ((kvInfo->DataLength % sizeof(WCHAR)) != 0)) { + DPRINT1("ObjectName invalid (Type = %lu, DataLength = %lu)\n", + kvInfo->Type, + kvInfo->DataLength); ExFreePool(kvInfo); continue; } - instancePath.Length = kvInfo->DataLength - sizeof(UNICODE_NULL); + instancePath.Length = (USHORT)(kvInfo->DataLength - sizeof(UNICODE_NULL)); instancePath.MaximumLength = kvInfo->DataLength; instancePath.Buffer = ExAllocatePoolWithTag(NonPagedPool, instancePath.MaximumLength, @@ -1948,13 +1960,19 @@ IopLoadDriver( Status = IopGetRegistryValue(ServiceHandle, L"ImagePath", &kvInfo); if (NT_SUCCESS(Status)) { - if ((kvInfo->Type != REG_EXPAND_SZ && kvInfo->Type != REG_SZ) || kvInfo->DataLength == 0) + if ((kvInfo->Type != REG_EXPAND_SZ && kvInfo->Type != REG_SZ) || + (kvInfo->DataLength < sizeof(UNICODE_NULL)) || + (kvInfo->DataLength > UNICODE_STRING_MAX_BYTES) || + ((kvInfo->DataLength % sizeof(WCHAR)) != 0)) { + DPRINT1("ObjectName invalid (Type = %lu, DataLength = %lu)\n", + kvInfo->Type, + kvInfo->DataLength); ExFreePool(kvInfo); return STATUS_ILL_FORMED_SERVICE_ENTRY; } - ImagePath.Length = kvInfo->DataLength - sizeof(UNICODE_NULL); + ImagePath.Length = (USHORT)(kvInfo->DataLength - sizeof(UNICODE_NULL)); ImagePath.MaximumLength = kvInfo->DataLength; ImagePath.Buffer = ExAllocatePoolWithTag(PagedPool, ImagePath.MaximumLength, TAG_RTLREGISTRY); if (!ImagePath.Buffer) diff --git a/ntoskrnl/io/pnpmgr/devaction.c b/ntoskrnl/io/pnpmgr/devaction.c index 94e384fac13..28079f76c6a 100644 --- a/ntoskrnl/io/pnpmgr/devaction.c +++ b/ntoskrnl/io/pnpmgr/devaction.c @@ -628,11 +628,14 @@ PiCallDriverAddDevice( Status = IopGetRegistryValue(SubKey, REGSTR_VAL_CLASSGUID, &kvInfo); if (NT_SUCCESS(Status)) { - if (kvInfo->Type == REG_SZ && kvInfo->DataLength > sizeof(WCHAR)) + if ((kvInfo->Type == REG_SZ) && + (kvInfo->DataLength > sizeof(UNICODE_NULL)) && + (kvInfo->DataLength <= UNICODE_STRING_MAX_BYTES) && + ((kvInfo->DataLength % sizeof(WCHAR)) == 0)) { UNICODE_STRING classGUID = { .MaximumLength = kvInfo->DataLength, - .Length = kvInfo->DataLength - sizeof(UNICODE_NULL), + .Length = (USHORT)(kvInfo->DataLength - sizeof(UNICODE_NULL)), .Buffer = (PVOID)((ULONG_PTR)kvInfo + kvInfo->DataOffset) }; HANDLE ccsControlHandle; @@ -1363,14 +1366,20 @@ IopSetServiceEnumData( return Status; } - if (kvInfo2->Type != REG_SZ || kvInfo2->DataLength <= sizeof(WCHAR)) + if ((kvInfo2->Type != REG_SZ) || + (kvInfo2->DataLength <= sizeof(UNICODE_NULL)) || + (kvInfo2->DataLength > UNICODE_STRING_MAX_BYTES) || + ((kvInfo2->DataLength % sizeof(WCHAR)) != 0)) { + DPRINT1("ObjectName invalid (Type = %lu, DataLength = %lu)\n", + kvInfo2->Type, + kvInfo2->DataLength); ExFreePool(kvInfo2); return STATUS_UNSUCCESSFUL; } ServiceName.MaximumLength = kvInfo2->DataLength; - ServiceName.Length = kvInfo2->DataLength - sizeof(UNICODE_NULL); + ServiceName.Length = (USHORT)(kvInfo2->DataLength - sizeof(UNICODE_NULL)); ServiceName.Buffer = (PVOID)((ULONG_PTR)kvInfo2 + kvInfo2->DataOffset); DPRINT("IopSetServiceEnumData(%p)\n", DeviceNode); @@ -1508,7 +1517,7 @@ done: * Sends IRP_MN_QUERY_PNP_DEVICE_STATE request and sets device node's flags * according to the result. * Tree reenumeration should be started upon a successful return of the function. - * + * * @todo Do not return STATUS_SUCCESS if nothing is changed. */ static @@ -1538,7 +1547,7 @@ PiUpdateDeviceState( if (PnPFlags & PNP_DEVICE_REMOVED || PnPFlags & PNP_DEVICE_DISABLED) { PiSetDevNodeProblem(DeviceNode, - PnPFlags & PNP_DEVICE_DISABLED + PnPFlags & PNP_DEVICE_DISABLED ? CM_PROB_HARDWARE_DISABLED : CM_PROB_DEVICE_NOT_THERE); @@ -2413,7 +2422,7 @@ PiDevNodeStateMachine( PiIrpQueryStopDevice(currentNode); PiSetDevNodeState(currentNode, DeviceNodeQueryStopped); } - + doProcessAgain = TRUE; } break; @@ -2617,7 +2626,7 @@ PipDeviceActionWorker( } } // TODO: Windows may return STATUS_DELETE_PENDING here - status = STATUS_SUCCESS; + status = STATUS_SUCCESS; break; default: diff --git a/ntoskrnl/ob/oblink.c b/ntoskrnl/ob/oblink.c index 47b813b802a..d359df6b390 100644 --- a/ntoskrnl/ob/oblink.c +++ b/ntoskrnl/ob/oblink.c @@ -442,7 +442,7 @@ ObpParseSymbolicLink(IN PVOID ParsedObject, POBJECT_SYMBOLIC_LINK SymlinkObject = (POBJECT_SYMBOLIC_LINK)ParsedObject; PUNICODE_STRING TargetPath; PWSTR NewTargetPath; - ULONG LengthUsed, MaximumLength, TempLength; + SIZE_T LengthUsed, MaximumLength, TempLength; NTSTATUS Status; PAGED_CODE();