[PATCHv3 5/5] xeniface: Bound NumberPermissions before checking the input length

[Césaire Mounah <cesaire.mounah@xxxxxxxxxx>]

IoctlStoreSetPermissions() compares InLen with
FIELD_OFFSET(XENIFACE_STORE_SET_PERMISSIONS_IN, Permissions[N]), whose
64-bit result is truncated to ULONG. The count was only capped to 255
later, in __ConvertPermissions(), so the length check alone could be
satisfied by a huge count.

Check the cap first, with the same STATUS_INVALID_PARAMETER as before.
The check in __ConvertPermissions() is removed and fails are updated
accordingly.

Signed-off-by: Césaire Mounah <cesaire.mounah@xxxxxxxxxx>
---
 src/xeniface/ioctl_store.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/xeniface/ioctl_store.c b/src/xeniface/ioctl_store.c
index 97bef75..d0c73ef 100644
--- a/src/xeniface/ioctl_store.c
+++ b/src/xeniface/ioctl_store.c
@@ -417,18 +417,21 @@ IoctlStoreSetPermissions(
         goto fail1;
     }
 
-    if (InLen != (ULONG)FIELD_OFFSET(XENIFACE_STORE_SET_PERMISSIONS_IN, 
Permissions[In->NumberPermissions]))
+    status = STATUS_INVALID_PARAMETER;
+    if (In->NumberPermissions > 255)
         goto fail2;
 
+    status = STATUS_INVALID_BUFFER_SIZE;
+    if (InLen != (ULONG)FIELD_OFFSET(XENIFACE_STORE_SET_PERMISSIONS_IN, 
Permissions[In->NumberPermissions]))
+        goto fail3;
+
     status = STATUS_INVALID_PARAMETER;
     if (In->PathLength == 0 ||
         In->PathLength > XENSTORE_ABS_PATH_MAX) {
-        goto fail3;
+        goto fail4;
     }
 
     Permissions = __ConvertPermissions(In->NumberPermissions, In->Permissions);
-    if (Permissions == NULL)
-        goto fail4;
 
     status = __CaptureUserBuffer(In->Path, In->PathLength, &Path);
     if (!NT_SUCCESS(status))
-- 
2.55.0.windows.5



--
Césaire Mounah | Vates Windows Guest Tools Engineer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech