Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulnerability found #544

Open
DEPazifist opened this issue Aug 23, 2024 · 0 comments
Open

Vulnerability found #544

DEPazifist opened this issue Aug 23, 2024 · 0 comments
Labels

Comments

@DEPazifist
Copy link

Hi, i am not a the position to write any drivers, but i also loved the Windows Precision Touchpad Implementation for Magic Trackpad 2 and use it for years on my private device. I asked my company if I could also use the drivers for my company device as well. Unfortunately no. They ran a Checkmarx SAST and found a vulnerability.
Unprocessed results are 4 highs, 1 low.
All 4 highs point at the same functionality (in 2 separate device.c files). The descriptions are
"The size from buffer in src\AmtPtpDeviceUsbUm\Device.c at line 447 does not correctly account for the actual size of the buffer, resulting in an incorrect access that is off by one."
"The array index buffer at src\AmtPtpDeviceUsbKm\Device.c in line 590 is used to reference an index of a cell of the array buffer at src\AmtPtpDeviceUsbKm\Device.c in line 590 in a way that may exceed array bounds."
Line 590 here https://github.com/imbushuo/mac-precision-touchpad/blob/master/src/AmtPtpDeviceUsbKm/Device.c
Line 447 here https://github.com/imbushuo/mac-precision-touchpad/blob/master/src/AmtPtpDeviceUsbUm/Device.c
Device.c

Vulnerability 1: Line 447

The code allocates memory based on DeviceContext->DeviceInfo->um_size. This value is retrieved from the registry and not directly controlled by the driver.
In WDF_MEMORY_DESCRIPTOR_INIT_BUFFER, the buffer size is set to sizeof(DeviceContext->DeviceInfo->um_size). This might be insufficient if the actual data size in the buffer is larger than the size of the structure member um_size.
If an attacker can manipulate the registry to set um_size to a larger value than the actual data size, there could be a buffer overflow when the code tries to access memory beyond the allocated buffer. This could potentially lead to code execution or system crashes.

Vulnerability 2: Line 590

This vulnerability is similar to the one at line 447. The code uses the same logic to allocate memory and set the buffer size in the memory descriptor. If the attacker can manipulate um_size in the registry, it can lead to a buffer overflow vulnerability.

Mitigation:
Here are some ways to mitigate these vulnerabilities:

Validate um_size: Before allocating memory or using the buffer size, validate the value of DeviceContext->DeviceInfo->um_size to ensure it is within a reasonable range.
Use appropriate buffer size: Instead of using sizeof(DeviceContext->DeviceInfo->um_size), use the actual data size expected in the buffer. This information might be available from the device configuration or the data format specification.

These mitigations can help prevent attackers from exploiting these vulnerabilities to compromise the system.

Is this a valid point and if yes, is someone able to cure?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant