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

Enforce that CANMessage.m_dataSize won't be larger than 8 bytes #40

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

NoahAndrews
Copy link
Member

@NoahAndrews NoahAndrews commented Oct 30, 2024

Previously, the actual data array in CANMessage was always limited to 8 bytes, but m_dataSize could be larger than that, which could trick consumers of this library into reading from an invalid memory location.

Marking this as a draft*, because I think CandleWinUsbDeviceThread suffers from a similar issue. The data field on candle_frame_t (which represents the actual format of the USB packet) always has a length of 8, but it's possible for the dlc sent over CAN to be higher than that. This line of code should not assume that frame.data is at least as large as frame.can_dlc:

memcpy(frame.data, el.m_msg.GetData(), frame.can_dlc);

We should also determine whether gs_usb devices populate frame.can_dlc values larger than 8 as their over-the-CAN-bus format of 4 bits, or whether they handle expanding that to the actual size (DLC values ranging from 1001 to 1111 are used to specify data lengths of 12, 16, 20, 24, 32, 48, and 64 bytes, respectively), as that will influence that logic.

*also, it doesn't currently compile

@NoahAndrews NoahAndrews marked this pull request as draft October 30, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant