opendroneid / opendroneid-core-c

Open Drone ID Core C Library
Apache License 2.0
170 stars 64 forks source link

encodeBasicIDMessage use of strncpy #75

Closed kc2rxo closed 1 year ago

kc2rxo commented 1 year ago
int encodeBasicIDMessage(ODID_BasicID_encoded *outEncoded, ODID_BasicID_data *inData)
{
    if (!outEncoded || !inData ||
        !intInRange(inData->IDType, 0, 15) ||
        !intInRange(inData->UAType, 0, 15))
        return ODID_FAIL;

    outEncoded->MessageType = ODID_MESSAGETYPE_BASIC_ID;
    outEncoded->ProtoVersion = ODID_PROTOCOL_VERSION;
    outEncoded->IDType = inData->IDType;
    outEncoded->UAType = inData->UAType;
    strncpy(outEncoded->UASID, inData->UASID, sizeof(outEncoded->UASID));
    memset(outEncoded->Reserved, 0, sizeof(outEncoded->Reserved));
    return ODID_SUCCESS;
}

Per F3411-22a the encoding and format of the UAS ID's are as follows:

  1. Serial Number: presumably ASCII, formatted as an ANSI/CTA 2063-A Serial Number
  2. CAA Assigned ID: presumably ASCII, formatted as <ICAO Nationality MarkA >.<CAA Assigned ID>
  3. UTM Assigned ID: presumably bytes, formatted as 128-bit UUIDv4
  4. Specific Session ID: bytes, with first byte being Specific Session ID Type and last 19 being defined by the first bytes specification.

Due to the behavior of strncpy [1] as currently written any Type 3 and Type 4 UAS IDs with a byte of value 0x00 will be truncated at that byte. For example a Type 4 of the value 0x0120010030112233058899AABBCCDDEEFF000000 will be encoded and displayed over air as 0x0120010000000000000000000000000000000000 as the 0x00 in byte position 4 will be treated as a null terminator by strncpy.

Either the strncpy should be swapped for memcpy to be agnostic to the null terminator (0x00) or have a clause based on the ID Type and use strncpy for Type 1 and Type 2, and memcpy for Type 3, Type 4 or other currently undefined ID Types.

I suspect that the decode side also treats the UAS ID as ASCII at all times but have yet to check and confirm this. I suspect a simple memcpy replacement there as well would suffice but have not analyzed any side-effects of such a change.

[1] https://en.cppreference.com/w/cpp/string/byte/strncpy

friissoren commented 1 year ago

You are correct. Both for encode and decode.

Apparently this was already fixed a while ago in the Android receiver application but no-one thought of fixing this in the core-c library.

Pull Requests are very welcome. Otherwise, let's see when I have a moment to look at this.