T09 · Insecure Skill Coding Practices
- Location
memory-embedded.md:193- Finding
Unsafe CAN DLC Handling Can Cause Out-of-Bounds Writes and Undefined Shifts
- Content
View full analysis
Vulnerability Details
File Location:
memory-embedded.md, lines 193–201
Vulnerability Type: Unsafe bounds handling in a safety-critical code-generation template
Risk Level: HighVulnerable Code:
c uint32_t raw_dlc = CAN1_sFIFOMailBox[0].RDTR & 0x0Fu; /* 2. Copy data to shared volatile buffer */ uint8_t i; for (i = 0u; i < (uint8_t)raw_dlc; i++) { g_can_rx_buf[i] = (uint8_t)(CAN1_sFIFOMailBox[0].RDLR >> (i * 8u)); } g_can_rx_id = raw_id >> 21u; /* extract CAN ID */ g_can_rx_dlc = (uint8_t)raw_dlc;Technical Analysis
The code masks the received data-length code to four bits but does not validate it against the CAN payload limit, the size of
g_can_rx_buf, or the number of bytes available in theRDLRregister.Consequently, a DLC greater than the destination capacity can make the loop write beyond
g_can_rx_buf. In addition,RDLRis a 32-bit value. Whenireaches 4, the expressioni * 8uproduces a shift count of 32. Shifting a 32-bit value by 32 or more is undefined behavior in C. The example also reads onlyRDLR, so it cannot correctly retrieve bytes beyond the first four.Because this appears under the prescriptive “Do in ISRs” section, an Agent may reproduce it in generated automotive code and incorrectly present it as a safe embedded pattern.
Attack Path
- The template is adopted in a CAN receive interrupt handler.
- A malformed frame, corrupted peripheral state, or unsupported DLC value produces a value above the valid payload or buffer limit.
- The loop processes the unvalidated DLC.
- For sufficiently large values, it writes past
g_can_rx_buf. - At
i >= 4, it also performs an invalid shift of at least 32 bits. - The resulting memory corruption or undefined behavior may alter adjacent state, trigger a fault, or disrupt ECU control flow.
Impact Assessment
The issue does not grant operating-system privileges by itself. I ...[truncated 473 chars]
- Remediation
View remediation
Remediation Suggestions
- Validate the DLC before entering the copy loop.
- Bound the effective length by the protocol maximum and
sizeof(g_can_rx_buf). - Reject invalid DLC values and record a communication fault rather than silently truncating safety-relevant input.
- Read bytes 0–3 from
RDLRand bytes 4–7 from the corresponding high-data register. - Ensure every shift count is strictly less than the width of the shifted operand.
- Prefer a documented hardware-abstraction-layer receive API where available.
- Add static-analysis checks and tests for DLC values 0, 4, 8, and every invalid value.
A safe pattern should first validate the length:
c uint32_t raw_dlc = CAN1_sFIFOMailBox[0].RDTR & 0x0Fu; uint8_t dlc = 0u; if (raw_dlc <= 8u) { dlc = (uint8_t)raw_dlc; } else { report_error(ERR_CAN_INVALID_DLC); } if (dlc <= (uint8_t)sizeof(g_can_rx_buf)) { /* Copy from both low and high data registers with shifts below 32. */ } else { report_error(ERR_CAN_RX_BUFFER_TOO_SMALL); }