search

aws / amazon-freertos

DEPRECATED - See README.md
https://aws.amazon.com/freertos/
MIT License
2.54k stars 1.1k forks source link

cbor decoder - getting last string in map with indefinite length has an offset of 1 byte #1915

Closed rvanputten closed 4 years ago

rvanputten commented 4 years ago

Describe the bug Some of the cbor responses from the aws cloud contain indefinite length maps. If the last value of the map is a text string (probably also byte string) when getting the value using the find function of the cbor decoder, the value has an offset of 1.

Example: This is the response I get from an invalid create cert request: BF 6A 73 74 61 74 75 73 43 6F 64 65 19 01 90 69 65 72 72 6F 72 43 6F 64 65 6E 49 6E 76 61 6C 69 64 50 61 79 6C 6F 61 64 6C 65 72 72 6F 72 4D 65 73 73 61 67 65 78 18 4D 65 73 73 61 67 65 20 63 61 6E 6E 6F 74 20 62 65 20 70 61 72 73 65 64 FF I checked the validity of the cbor using http://cbor.me/. The JSON representation is this: {"statusCode": 400, "errorCode": "InvalidPayload", "errorMessage": "Message cannot be parsed"}

When I print the errorMessage from my code, I get this: essage cannot be parsed�

If a map doesn't specify the length upfront but uses an indefinite length map https://tools.ietf.org/html/rfc7049#section-2.2, a special break character is inserted after the values of the map: https://tools.ietf.org/html/rfc7049#section-2.2.1

I think I've narrowed down the problem to this line: https://github.com/aws/amazon-freertos/blob/master/libraries/c_sdk/standard/serializer/src/cbor/iot_serializer_tinycbor_decoder.c#L251 which doesn't take the extra break character into account.

System information

Expected behavior The strings is returned complete and without offset: Message cannot be parsed

Screenshots or console output Actual result: essage cannot be parsed� As you can see, the first byte is missing and the break character is printed

Code to reproduce the bug I've removed error checking for readability but none of the calls give an error.

void testFunction()
{
    uint8_t payload[] = {0xBF ,0x6A ,0x73 ,0x74 ,0x61 ,0x74 ,0x75 ,0x73 ,0x43 ,0x6F ,0x64 ,0x65 ,0x19 ,0x01 ,0x90 ,0x69 ,0x65 ,0x72 ,0x72 ,0x6F ,0x72 ,0x43 ,0x6F ,0x64 ,0x65 ,0x6E ,0x49 ,0x6E ,0x76 ,0x61 ,0x6C ,0x69 ,0x64 ,0x50 ,0x61 ,0x79 ,0x6C ,0x6F ,0x61 ,0x64 ,0x6C ,0x65 ,0x72 ,0x72 ,0x6F ,0x72 ,0x4D ,0x65 ,0x73 ,0x73 ,0x61 ,0x67 ,0x65 ,0x78 ,0x18 ,0x4D ,0x65 ,0x73 ,0x73 ,0x61 ,0x67 ,0x65 ,0x20 ,0x63 ,0x61 ,0x6E ,0x6E ,0x6F ,0x74 ,0x20 ,0x62 ,0x65 ,0x20 ,0x70 ,0x61 ,0x72 ,0x73 ,0x65 ,0x64 ,0xFF};
    IotSerializerDecoderObject_t decoderObj = IOT_SERIALIZER_DECODER_OBJECT_INITIALIZER;
    _IotSerializerCborDecoder.init(&decoderObj, payload, sizeof(payload));
    IotSerializerDecoderObject_t errorMessage = IOT_SERIALIZER_DECODER_OBJECT_INITIALIZER;
    _IotSerializerCborDecoder.find(&decoderObj, "errorMessage", &errorMessage);
    IotLogError("errorMessage type:%i, val:%.*s", errorMessage.type, errorMessage.u.value.u.string.length, errorMessage.u.value.u.string.pString);
}
sameerdasarwad commented 4 years ago

if you provide logs may be i can help, look like two errors there

rvanputten commented 4 years ago

@sameerdasarwad What logs are you looking for? I don't see any print statements in the cbor decode code and I don't see how other logs are relevant. Which two errors do you see?

qiutongs commented 4 years ago

@rvanputten Thank you for reporting this bug and providing detailed information. I agree all you said. And the culprit is this line as you mentioned. https://github.com/aws/amazon-freertos/blob/master/libraries/c_sdk/standard/serializer/src/cbor/iot_serializer_tinycbor_decoder.c#L251

We are trying to figure out a good fix.

rvanputten commented 4 years ago

@qiutongs Thanks for picking this up! I'll keep an eye on the fix to see when my workaround can be removed. This is what I currently do but while this may work for text strings, a last byte of 0xFF can be valid for byte strings:

char *value = (char *)valueDecoder.u.value.u.string.pString;
size_t length = valueDecoder.u.value.u.string.length;
// Check for bug in cbor decoder: https://github.com/aws/amazon-freertos/issues/1915
if (value[length - 1] == 0xFF)
    value--;
qiutongs commented 4 years ago

@qiutongs Thanks for picking this up! I'll keep an eye on the fix to see when my workaround can be removed. This is what I currently do but while this may work for text strings, a last byte of 0xFF can be valid for byte strings:

char *value = (char *)valueDecoder.u.value.u.string.pString;
size_t length = valueDecoder.u.value.u.string.length;
// Check for bug in cbor decoder: https://github.com/aws/amazon-freertos/issues/1915
if (value[length - 1] == 0xFF)
    value--;

I thought about the same workaround and I agree that the concern is the a valid 0xFF byte for byte strings.

qiutongs commented 4 years ago

@rvanputten The fix in #1955 should be good. You can copy it to your project and remove your workaround. I closed the the PR #1955 because we have to put the fix to C-SDK first and merge from C-SDK to amazon-freertos later. If you really need the fix in amazon-freertos very soon, please let us know.

qiutongs commented 4 years ago

Please let me us know if you have other question.