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

Improve HID report parsing and safety checks #219

Merged
merged 1 commit into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/hid_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,11 @@ void store_element(parser_state_t *parser, report_val_t *val, int i, uint32_t da
}

void handle_global_item(parser_state_t *parser, item_t *item) {
if (item->hdr.tag == RI_GLOBAL_REPORT_ID)
if (item->hdr.tag == RI_GLOBAL_REPORT_ID) {
// reset offset for a new page
parser->offset_in_bits = 0;
parser->report_id = item->val;
}

parser->globals[item->hdr.tag] = *item;
}
Expand Down
5 changes: 4 additions & 1 deletion src/hid_report.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include "main.h"

/* Given a value struct with size and offset in bits, find and return a value from the HID report */
int32_t get_report_value(uint8_t *report, report_val_t *val) {
int32_t get_report_value(uint8_t *report, int len, report_val_t *val) {
/* Calculate the bit offset within the byte */
uint16_t offset_in_bits = val->offset % 8;

Expand All @@ -28,6 +28,9 @@ int32_t get_report_value(uint8_t *report, report_val_t *val) {

/* Calculate the byte offset in the array */
uint16_t byte_offset = val->offset >> 3;

if (byte_offset >= len)
return 0;

/* Create a mask for the specified number of bits */
uint32_t mask = (1u << val->size) - 1;
Expand Down
2 changes: 1 addition & 1 deletion src/include/main.h
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ bool tud_mouse_report(uint8_t mode, uint8_t buttons, int16_t x, int16_t y, int8_
void process_mouse_report(uint8_t *, int, uint8_t, hid_interface_t *);
void parse_report_descriptor(hid_interface_t *, uint8_t const *, int);
void extract_data(hid_interface_t *, report_val_t *);
int32_t get_report_value(uint8_t *report, report_val_t *val);
int32_t get_report_value(uint8_t *report, int len, report_val_t *val);
void queue_mouse_report(mouse_report_t *, device_t *);
void output_mouse_report(mouse_report_t *, device_t *);

Expand Down
34 changes: 26 additions & 8 deletions src/mouse.c
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ void do_screen_switch(device_t *state, int direction) {
switch_virtual_desktop(state, output, output->screen_index + 1, direction);
}

void extract_report_values(uint8_t *raw_report, device_t *state, mouse_values_t *values, hid_interface_t *iface) {
void extract_report_values(uint8_t *raw_report, int len, device_t *state, mouse_values_t *values, hid_interface_t *iface) {
/* Interpret values depending on the current protocol used. */
if (iface->protocol == HID_PROTOCOL_BOOT) {
hid_mouse_report_t *mouse_report = (hid_mouse_report_t *)raw_report;
Expand All @@ -239,14 +239,32 @@ void extract_report_values(uint8_t *raw_report, device_t *state, mouse_values_t
}

/* If HID Report ID is used, the report is prefixed by the report ID so we have to move by 1 byte */
if (iface->mouse.report_id)
if (iface->uses_report_id) {
uint8_t report_id = raw_report[0];
raw_report++;
if (report_id == iface->mouse.move_x.report_id)
values->move_x = get_report_value(raw_report, len, &iface->mouse.move_x);

values->move_x = get_report_value(raw_report, &iface->mouse.move_x);
values->move_y = get_report_value(raw_report, &iface->mouse.move_y);
values->wheel = get_report_value(raw_report, &iface->mouse.wheel);
values->pan = get_report_value(raw_report, &iface->mouse.pan);
values->buttons = get_report_value(raw_report, &iface->mouse.buttons);
if (report_id == iface->mouse.move_y.report_id)
values->move_y = get_report_value(raw_report, len, &iface->mouse.move_y);

if (report_id == iface->mouse.wheel.report_id)
values->wheel = get_report_value(raw_report, len, &iface->mouse.wheel);

if (report_id == iface->mouse.pan.report_id)
values->pan = get_report_value(raw_report, len, &iface->mouse.pan);

if (report_id == iface->mouse.buttons.report_id)
values->buttons = get_report_value(raw_report, len, &iface->mouse.buttons);
else
values->buttons = state->mouse_buttons;
} else {
values->move_x = get_report_value(raw_report, len, &iface->mouse.move_x);
values->move_y = get_report_value(raw_report, len, &iface->mouse.move_y);
values->wheel = get_report_value(raw_report, len, &iface->mouse.wheel);
values->pan = get_report_value(raw_report, len, &iface->mouse.pan);
values->buttons = get_report_value(raw_report, len, &iface->mouse.buttons);
}
}

mouse_report_t create_mouse_report(device_t *state, mouse_values_t *values) {
Expand Down Expand Up @@ -274,7 +292,7 @@ void process_mouse_report(uint8_t *raw_report, int len, uint8_t itf, hid_interfa
device_t *state = &global_state;

/* Interpret the mouse HID report, extract and save values we need. */
extract_report_values(raw_report, state, &values, iface);
extract_report_values(raw_report, len, state, &values, iface);

/* Calculate and update mouse pointer movement. */
enum screen_pos_e switch_direction = update_mouse_position(state, &values);
Expand Down