matthiasbock / cantact-fw

Firmware source files for the CANtact USB-CAN-Bus-Adapter
http://cantact.io
Other
2 stars 1 forks source link

Possible string index error during SLCAN frame parsing #3

Closed matthiasbock closed 6 years ago

matthiasbock commented 6 years ago

Commit: 2c134e8c17ae35662a07742602fc836b6df9f45d

Location of possible implementation error: https://github.com/matthiasbock/cantact-fw/blob/2c134e8c17ae35662a07742602fc836b6df9f45d/Src/slcan.c#L206

    // Parser position in the buffer
    uint8_t i = 0;

    frame->IDE = ((buffer[i] == SLCAN_TRANSMIT_STANDARD) || (buffer[i] == SLCAN_TRANSMIT_REQUEST_STANDARD))
                    ? CAN_ID_STD
                    : CAN_ID_EXT;

    frame->RTR = ((buffer[i] == SLCAN_TRANSMIT_REQUEST_STANDARD) || (buffer[i] == SLCAN_TRANSMIT_REQUEST_EXTENDED))
                    ? CAN_RTR_REMOTE
                    : CAN_RTR_DATA;

    frame->StdId = 0;
    frame->ExtId = 0;
    if (frame->IDE == CAN_ID_STD) {
        // Parse hexadecimal representation of standard CAN ID
        for (uint8_t j=i; j <= SLCAN_STD_ID_LEN; j++, i++) {
            frame->StdId <<= 4;
            frame->StdId += hex2int(buffer[j]);
        }
    } else {
        // Parse hexadecimal representation of extended CAN ID
        for (uint8_t j=i; j <= SLCAN_EXT_ID_LEN; j++, i++) {
            frame->ExtId <<= 4;
            frame->ExtId += hex2int(buffer[j]);
        }
    }

i is initialized as 0 and not incremented until the ID parsing for loops for standard or extended ID. These loops however must start with i=1, instead of 0.

Surprisingly, the firmware compiled from this commit appears to be functional, i.e. receives and transmits CAN frames correctly.

matthiasbock commented 6 years ago

Code apparently doesn't cause any problems, so this issue can be closed.