From 9973fff1619700d64b89f391e6988968ac0196df Mon Sep 17 00:00:00 2001 From: Ramon Roche Date: Mon, 6 Apr 2026 20:21:09 -0700 Subject: [PATCH] fix(sagetech_mxs): bounds-check SVR decoder against truncated packets The Sagetech SVR decoder walked optional fields based on a flag mask in the packet header without ever checking the available buffer length, so a truncated state vector report could read past the end of the received buffer. Pass the valid byte length from the framer into sgDecodeSVR and verify the required size before each optional field read. The decoder now bails out cleanly on a short packet instead of continuing past the end of the buffer. The single caller in SagetechMXS::handle_packet supplies the framed length (packet header plus payload). Refs: GHSA-q4m8-7h6g-vj3r Signed-off-by: Ramon Roche --- .../transponder/sagetech_mxs/SagetechMXS.cpp | 5 +++- .../transponder/sagetech_mxs/sg_sdk/sg.h | 6 +++-- .../sagetech_mxs/sg_sdk/sgDecodeSVR.c | 26 ++++++++++++++++++- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/drivers/transponder/sagetech_mxs/SagetechMXS.cpp b/src/drivers/transponder/sagetech_mxs/SagetechMXS.cpp index 77e373526d..4c250a19fc 100644 --- a/src/drivers/transponder/sagetech_mxs/SagetechMXS.cpp +++ b/src/drivers/transponder/sagetech_mxs/SagetechMXS.cpp @@ -903,7 +903,10 @@ void SagetechMXS::handle_packet(const Packet &msg) case MsgType::ADSB_StateVector_Report: { sg_svr_t svr{}; - if (sgDecodeSVR((uint8_t *) &msg, &svr)) { + // pass total valid byte length (4-byte header + payload) so the decoder can bounds-check + const size_t svr_len = (size_t)msg.payload_length + 4u; + + if (sgDecodeSVR((uint8_t *) &msg, svr_len, &svr)) { handle_svr(svr); } diff --git a/src/drivers/transponder/sagetech_mxs/sg_sdk/sg.h b/src/drivers/transponder/sagetech_mxs/sg_sdk/sg.h index b5e3ce6ced..378a06bafe 100644 --- a/src/drivers/transponder/sagetech_mxs/sg_sdk/sg.h +++ b/src/drivers/transponder/sagetech_mxs/sg_sdk/sg.h @@ -18,6 +18,7 @@ #ifndef SG_H #define SG_H +#include #include #include @@ -880,11 +881,12 @@ bool sgDecodeFlightId(uint8_t *buffer, sg_flightid_t *id); * Process the state vector report message. * * @param[in] buffer The raw SVR message buffer. + * @param[in] len Total length of the buffer in bytes (including header). * @param[out] svr The parsed SVR message. * - * @return true if successful or false on failure. + * @return true if successful or false on failure / truncated input. */ -bool sgDecodeSVR(uint8_t *buffer, sg_svr_t *svr); +bool sgDecodeSVR(uint8_t *buffer, size_t len, sg_svr_t *svr); /** * Process the mode status report message. diff --git a/src/drivers/transponder/sagetech_mxs/sg_sdk/sgDecodeSVR.c b/src/drivers/transponder/sagetech_mxs/sg_sdk/sgDecodeSVR.c index 3d24a7729d..591db4f116 100644 --- a/src/drivers/transponder/sagetech_mxs/sg_sdk/sgDecodeSVR.c +++ b/src/drivers/transponder/sagetech_mxs/sg_sdk/sgDecodeSVR.c @@ -47,10 +47,17 @@ /* * Documented in the header file. */ -bool sgDecodeSVR(uint8_t *buffer, sg_svr_t *svr) +#define NEED(n) do { if ((size_t)PBASE + (size_t)ofs + (size_t)(n) > len) { return false; } } while (0) + +bool sgDecodeSVR(uint8_t *buffer, size_t len, sg_svr_t *svr) { // memset(svr, 0, sizeof(sg_svr_t)); + // minimum mandatory header: PBASE + 9 bytes (fields[0..2], flags, eflags, addr, addrType) + if (len < (size_t)PBASE + 9) { + return false; + } + uint8_t fields[3]; memcpy(&fields, &buffer[PBASE + 0], 3); @@ -63,21 +70,25 @@ bool sgDecodeSVR(uint8_t *buffer, sg_svr_t *svr) uint8_t ofs = 9; if (fields[0] & SV_PARAM_TOA_EPOS) { + NEED(2); svr->toaEst = toTOA(&buffer[PBASE + ofs]); ofs += 2; } if (fields[0] & SV_PARAM_TOA_POS) { + NEED(2); svr->toaPosition = toTOA(&buffer[PBASE + ofs]); ofs += 2; } if (fields[0] & SV_PARAM_TOA_VEL) { + NEED(2); svr->toaSpeed = toTOA(&buffer[PBASE + ofs]); ofs += 2; } if (fields[0] & SV_PARAM_LATLON) { + NEED(6); if (svr->validity.position) { svr->lat = toLatLon(&buffer[PBASE + ofs + 0]); svr->lon = toLatLon(&buffer[PBASE + ofs + 3]); @@ -92,6 +103,7 @@ bool sgDecodeSVR(uint8_t *buffer, sg_svr_t *svr) if (svr->type == svrAirborne) { if (fields[1] & SV_PARAM_GEOALT) { + NEED(3); if (svr->validity.geoAlt) { svr->airborne.geoAlt = toAlt(&buffer[PBASE + ofs]); @@ -103,6 +115,7 @@ bool sgDecodeSVR(uint8_t *buffer, sg_svr_t *svr) } if (fields[1] & SV_PARAM_VEL) { + NEED(4); if (svr->validity.airSpeed) { int16_t nvel = toVel(&buffer[PBASE + ofs + 0]); int16_t evel = toVel(&buffer[PBASE + ofs + 2]); @@ -123,6 +136,7 @@ bool sgDecodeSVR(uint8_t *buffer, sg_svr_t *svr) } if (fields[1] & SV_PARAM_BAROALT) { + NEED(3); if (svr->validity.baroAlt) { svr->airborne.baroAlt = toAlt(&buffer[PBASE + ofs]); @@ -134,6 +148,7 @@ bool sgDecodeSVR(uint8_t *buffer, sg_svr_t *svr) } if (fields[1] & SV_PARAM_VRATE) { + NEED(2); if (svr->validity.baroVRate || svr->validity.geoVRate) { svr->airborne.vrate = toInt16(&buffer[PBASE + ofs]); @@ -146,6 +161,7 @@ bool sgDecodeSVR(uint8_t *buffer, sg_svr_t *svr) } else { if (fields[1] & SV_PARAM_SURF_GS) { + NEED(1); if (svr->validity.surfSpeed) { svr->surface.speed = toGS(&buffer[PBASE + ofs]); @@ -157,6 +173,7 @@ bool sgDecodeSVR(uint8_t *buffer, sg_svr_t *svr) } if (fields[1] & SV_PARAM_SURF_HEAD) { + NEED(1); if (svr->validity.surfHeading) { svr->surface.heading = toHeading(&buffer[PBASE + ofs]); @@ -169,12 +186,14 @@ bool sgDecodeSVR(uint8_t *buffer, sg_svr_t *svr) } if (fields[1] & SV_PARAM_NIC) { + NEED(1); svr->nic = buffer[PBASE + ofs]; ofs += 1; } if (fields[1] & SV_PARAM_ESTLAT) { + NEED(3); if (svr->evalidity.estPosition) { svr->airborne.estLat = toLatLon(&buffer[PBASE + ofs]); @@ -186,6 +205,7 @@ bool sgDecodeSVR(uint8_t *buffer, sg_svr_t *svr) } if (fields[2] & SV_PARAM_ESTLON) { + NEED(3); if (svr->evalidity.estPosition) { svr->airborne.estLon = toLatLon(&buffer[PBASE + ofs]); @@ -197,13 +217,17 @@ bool sgDecodeSVR(uint8_t *buffer, sg_svr_t *svr) } if (fields[2] & SV_PARAM_SURV) { + NEED(1); svr->survStatus = buffer[PBASE + ofs]; ofs += 1; } if (fields[2] & SV_PARAM_REPORT) { + NEED(1); svr->mode = buffer[PBASE + ofs]; } return true; } + +#undef NEED