Closed Bug 1921198 Opened 7 months ago Closed 7 months ago

Crash in [@ memcpy | sbiedll.dll | GetRawInputDeviceInfo] caused by sandboxie

Categories

(External Software Affecting Firefox :: Other, defect)

Product:
External Software Affecting Firefox
Component:
Other
Platform:
Unspecified
Windows
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED INVALID

People

(Reporter: gsvelto, Unassigned)

Details

(Keywords: crash)

Crash Data

Crash report: https://crash-stats.mozilla.org/report/index/31e58f9a-cef0-4660-b810-939e90240925

Reason:

EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames:

0  ntdll.dll  memcpy
1  SbieDll.dll  SbieDll.dll@0x46748
2  xul.dll  GetRawInputDeviceInfo(void*, unsigned int, void*, unsigned int*)  dist/stl_wrappers/windows.h:5230
2  xul.dll  (anonymous namespace)::GetPreparsedData(void*, nsTArray<unsigned char>&)  dom/gamepad/windows/WindowsGamepad.cpp:215
3  xul.dll  (anonymous namespace)::WindowsGamepadService::HandleRawInput(HRAWINPUT__*)  dom/gamepad/windows/WindowsGamepad.cpp:812
3  xul.dll  (anonymous namespace)::GamepadWindowProc(HWND__*, unsigned int, unsigned long...  dom/gamepad/windows/WindowsGamepad.cpp:1025
4  user32.dll  UserCallWinProcCheckWow(_ACTIVATION_CONTEXT*, int64_t (*)(tagWND*, unsigned i...
5  user32.dll  CallWindowProcW
6  SbieDll.dll  SbieDll.dll@0x3e122
7  user32.dll  UserCallWinProcCheckWow(_ACTIVATION_CONTEXT*, int64_t (*)(tagWND*, unsigned i...

These appear to be new crashes caused by sandboxie. Worryingly a significant portion of them is hitting a guard page.

So... Looking at Sandboxie's implementation of GetRawInputDeviceInfo() (MSDN), we can see the issue: They assume that the pcbSize argument is always the number of wchar_t characters... But that's only correct if uiCommand == RIDI_DEVICENAME. For the other 2 enum values, it's the size (in bytes) of the corresponding struct.

(Admittedly, this fact is kind-of hidden at the bottom of the "Value-Meaning" table entry on MSDN. Such an important detail should probably be displayed more prominently.)

So Sandboxie is reading twice as much data as it should on Line 1348, and writing twice as much data as it should on Line 1368 when uiCommand != RIDI_DEVICENAME .

Another note Sandboxie also reads from *pcbSize on Line 1352, even if pData == nullptr. Since we don't initialize pcbSize at our callsite (here), this is also UB.

Filed an issue for Sandboxie.

Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.