Replying to this email means your email address will be shared with the team that works on this product.
[email protected] added comment #3:
Thanks for taking a look!
We have actually an auto-generated patch by CodeMender (our experimental AI agent for fixing vulnerabilities) for this issue.
These patches go through extensive automated validation to ensure that they're fixing the correct root cause and that they don't break any functionality. In addition to this, they are manually vetted by engineers before we send them, who will be able to address feedback and answer questions.
Would you mind taking a look? Is this a legitimate fix despite the fuzzer not being written correctly?
Let us know if you would prefer us to create a merge request for this.
Description:
driver_greis: fix buffer overflow in greis_msg_SI()
The GREIS driver failed to validate the satellite count derived from the
packet length in `greis_msg_SI()`. The code calculated
`satellites_visible` as `len - 1` without checking against `MAXCHANNELS`.
When a GREIS SI packet with a large length (e.g., 392) is processed, the
subsequent loop writing to `session->gpsdata.skyview` overflows the
array. Since `skyview` is a member of `gps_data_t` (the first member of
`gps_device_t`), this overflow corrupts adjacent members in the session
structure.
Specifically, this was observed corrupting `session->last_controller`.
The corruption leads to a segmentation fault in `gpsd_poll()` when the
`STICKY()` macro attempts to dereference the corrupted
`last_controller` pointer.
Fix this by clamping `satellites_visible` to `MAXCHANNELS` and logging a
warning if the packet exceeds the limit.
Patch:
diff --git a/drivers/driver_greis.c b/drivers/driver_greis.c
index c3c6975..d1cfffb 100644
--- a/drivers/driver_greis.c
+++ b/drivers/driver_greis.c
@@ -366,8 +366,13 @@
}
gpsd_zero_satellites(&session->gpsdata);
- // FIXME: check against MAXCHANNELS?
- session->gpsdata.satellites_visible = len - 1;
+ session->gpsdata.satellites_visible = (int)(len - 1);
+ if (session->gpsdata.satellites_visible > MAXCHANNELS) {
+ GPSD_LOG(LOG_WARN, &session->context->errout,
+ "GREIS: SI too many satellites %d\n",
+ session->gpsdata.satellites_visible);
+ session->gpsdata.satellites_visible = MAXCHANNELS;
+ }
for (i = 0; i < session->gpsdata.satellites_visible; i++) {
// This isn't really PRN, this is USI. Convert it.
unsigned short PRN = getub(buf, i);
Generated by Google IssueTracker notification system.
