From 1c64d45b3190cc61f65c7d68ea42acea0e7ec892 Mon Sep 17 00:00:00 2001 From: Lucio Torre Date: Wed, 30 Mar 2016 14:24:01 +0200 Subject: [PATCH 1/6] cbor: introduce bounds checking on read --- sys/cbor/cbor.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/sys/cbor/cbor.c b/sys/cbor/cbor.c index fd7b489690b0..e3de8b1923f8 100644 --- a/sys/cbor/cbor.c +++ b/sys/cbor/cbor.c @@ -78,6 +78,10 @@ if (stream->pos + bytes >= stream->size) { return 0; } \ } while(0) +#define CBOR_ENSURE_SIZE_READ(stream, bytes) do { \ + if (bytes > stream->size) { return 0; } \ +} while(0) + /* Extra defines not related to the protocol itself */ #define CBOR_STREAM_PRINT_BUFFERSIZE 1024 /* bytes */ @@ -323,10 +327,14 @@ static size_t decode_int(const cbor_stream_t *s, size_t offset, uint64_t *val) *val = 0; /* clear val first */ + CBOR_ENSURE_SIZE_READ(s, offset + 1); + unsigned char *in = &s->data[offset]; unsigned char additional_info = CBOR_ADDITIONAL_INFO(s, offset); unsigned char bytes_follow = uint_bytes_follow(additional_info); + CBOR_ENSURE_SIZE_READ(s, offset + 1 + bytes_follow); + switch (bytes_follow) { case 0: *val = (in[0] & CBOR_INFO_MASK); @@ -371,6 +379,8 @@ static size_t encode_bytes(unsigned char major_type, cbor_stream_t *s, const cha static size_t decode_bytes(const cbor_stream_t *s, size_t offset, char *out, size_t length) { + CBOR_ENSURE_SIZE_READ(s, offset + 1); + if ((CBOR_TYPE(s, offset) != CBOR_BYTES && CBOR_TYPE(s, offset) != CBOR_TEXT) || !out) { return 0; } @@ -386,6 +396,8 @@ static size_t decode_bytes(const cbor_stream_t *s, size_t offset, char *out, siz return 0; } + CBOR_ENSURE_SIZE_READ(s, offset + bytes_start + bytes_length); + memcpy(out, &s->data[offset + bytes_start], bytes_length); out[bytes_length] = '\0'; return (bytes_start + bytes_length); @@ -393,6 +405,8 @@ static size_t decode_bytes(const cbor_stream_t *s, size_t offset, char *out, siz size_t cbor_deserialize_int(const cbor_stream_t *stream, size_t offset, int *val) { + CBOR_ENSURE_SIZE_READ(stream, offset + 1); + if ((CBOR_TYPE(stream, offset) != CBOR_UINT && CBOR_TYPE(stream, offset) != CBOR_NEGINT) || !val) { return 0; } @@ -540,6 +554,8 @@ size_t cbor_serialize_float(cbor_stream_t *s, float val) size_t cbor_deserialize_double(const cbor_stream_t *stream, size_t offset, double *val) { + CBOR_ENSURE_SIZE_READ(stream, offset + 1); + if (CBOR_TYPE(stream, offset) != CBOR_7 || !val) { return 0; } @@ -547,6 +563,7 @@ size_t cbor_deserialize_double(const cbor_stream_t *stream, size_t offset, doubl unsigned char *data = &stream->data[offset]; if (*data == CBOR_FLOAT64) { + CBOR_ENSURE_SIZE_READ(stream, offset + 9); *val = ntohd(*(uint64_t *)(data + 1)); return 9; } @@ -568,6 +585,8 @@ size_t cbor_serialize_double(cbor_stream_t *s, double val) size_t cbor_deserialize_byte_string(const cbor_stream_t *stream, size_t offset, char *val, size_t length) { + CBOR_ENSURE_SIZE_READ(stream, offset + 1); + if (CBOR_TYPE(stream, offset) != CBOR_BYTES) { return 0; } @@ -588,6 +607,8 @@ size_t cbor_serialize_byte_stringl(cbor_stream_t *stream, const char *val, size_ size_t cbor_deserialize_unicode_string(const cbor_stream_t *stream, size_t offset, char *val, size_t length) { + CBOR_ENSURE_SIZE_READ(stream, offset + 1); + if (CBOR_TYPE(stream, offset) != CBOR_TEXT) { return 0; } From d783d784c9f86bed13c26542ea83f82494c36d26 Mon Sep 17 00:00:00 2001 From: Lucio Torre Date: Wed, 30 Mar 2016 14:28:55 +0200 Subject: [PATCH 2/6] cbor: add zero copy string deserialization --- sys/cbor/cbor.c | 52 +++++++++++++++++-- sys/include/cbor.h | 26 ++++++---- tests/unittests/tests-cbor/tests-cbor.c | 68 +++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 14 deletions(-) diff --git a/sys/cbor/cbor.c b/sys/cbor/cbor.c index e3de8b1923f8..cc50b4117592 100644 --- a/sys/cbor/cbor.c +++ b/sys/cbor/cbor.c @@ -403,6 +403,31 @@ static size_t decode_bytes(const cbor_stream_t *s, size_t offset, char *out, siz return (bytes_start + bytes_length); } +/* A zero copy version of decode_bytes. + Will not null termiante input, but tell you the size of what you read. + Great for reading byte strings which could contain nulls inside +*/ +static size_t decode_bytes_no_copy(const cbor_stream_t *s, size_t offset, unsigned char **out, size_t *length) +{ + CBOR_ENSURE_SIZE_READ(s, offset + 1); + + if ((CBOR_TYPE(s, offset) != CBOR_BYTES && CBOR_TYPE(s, offset) != CBOR_TEXT) || !out) { + return 0; + } + + uint64_t bytes_length; + size_t bytes_start = decode_int(s, offset, &bytes_length); + + if (!bytes_start) { + return 0; + } + + CBOR_ENSURE_SIZE_READ(s, offset + bytes_start + bytes_length); + *out = &(s->data[offset + bytes_start]); + *length = bytes_length; + return (bytes_start + bytes_length); +} + size_t cbor_deserialize_int(const cbor_stream_t *stream, size_t offset, int *val) { CBOR_ENSURE_SIZE_READ(stream, offset + 1); @@ -594,14 +619,21 @@ size_t cbor_deserialize_byte_string(const cbor_stream_t *stream, size_t offset, return decode_bytes(stream, offset, val, length); } -size_t cbor_serialize_byte_string(cbor_stream_t *stream, const char *val) +size_t cbor_deserialize_byte_string_no_copy(const cbor_stream_t *stream, size_t offset, unsigned char **val, + size_t *length) { - return encode_bytes(CBOR_BYTES, stream, val, strlen(val)); + CBOR_ENSURE_SIZE_READ(stream, offset + 1); + + if (CBOR_TYPE(stream, offset) != CBOR_BYTES) { + return 0; + } + + return decode_bytes_no_copy(stream, offset, val, length); } -size_t cbor_serialize_byte_stringl(cbor_stream_t *stream, const char *val, size_t length) +size_t cbor_serialize_byte_string(cbor_stream_t *stream, const char *val) { - return encode_bytes(CBOR_BYTES, stream, val, length); + return encode_bytes(CBOR_BYTES, stream, val, strlen(val)); } size_t cbor_deserialize_unicode_string(const cbor_stream_t *stream, size_t offset, char *val, @@ -616,6 +648,18 @@ size_t cbor_deserialize_unicode_string(const cbor_stream_t *stream, size_t offse return decode_bytes(stream, offset, val, length); } +size_t cbor_deserialize_unicode_string_no_copy(const cbor_stream_t *stream, size_t offset, unsigned char **val, + size_t *length) +{ + CBOR_ENSURE_SIZE_READ(stream, offset + 1); + + if (CBOR_TYPE(stream, offset) != CBOR_TEXT) { + return 0; + } + + return decode_bytes_no_copy(stream, offset, val, length); +} + size_t cbor_serialize_unicode_string(cbor_stream_t *stream, const char *val) { return encode_bytes(CBOR_TEXT, stream, val, strlen(val)); diff --git a/sys/include/cbor.h b/sys/include/cbor.h index 9b59dbf9720b..31400ce83e0a 100644 --- a/sys/include/cbor.h +++ b/sys/include/cbor.h @@ -365,16 +365,6 @@ size_t cbor_deserialize_double(const cbor_stream_t *stream, size_t offset, */ size_t cbor_serialize_byte_string(cbor_stream_t *stream, const char *val); -/** - * @brief Serializes an arbitrary byte string - * - * @param[out] stream The destination stream for serializing the byte stream - * @param[in] val The arbitrary byte string which may include null bytes - * @param[in] length The size of the byte string in bytes - * - * @return Number of bytes written to stream @p stream - */ -size_t cbor_serialize_byte_stringl(cbor_stream_t *stream, const char *val, size_t length); /** * @brief Deserialize bytes from @p stream to @p val @@ -391,6 +381,22 @@ size_t cbor_deserialize_byte_string(const cbor_stream_t *stream, size_t offset, size_t cbor_serialize_unicode_string(cbor_stream_t *stream, const char *val); +/** + * @brief Deserialize bytes/unicode from @p stream to @p val (without copy) + * + * @param[in] stream The stream to deserialize + * @param[in] offset The offset within the stream where to start deserializing + * @param[out] val Pointer to a char * + * @param[out] length Pointer tp a size_t to store the size of the string + * + * @return Number of bytes written into @p val + */ +size_t cbor_deserialize_byte_string_no_copy(const cbor_stream_t *stream, size_t offset, + unsigned char **val, size_t *length); + +size_t cbor_deserialize_unicode_string_no_copy(const cbor_stream_t *stream, size_t offset, + unsigned char **val, size_t *length); + /** * @brief Deserialize unicode string from @p stream to @p val * diff --git a/tests/unittests/tests-cbor/tests-cbor.c b/tests/unittests/tests-cbor/tests-cbor.c index f98cb8cf5559..76e2a7f744d8 100644 --- a/tests/unittests/tests-cbor/tests-cbor.c +++ b/tests/unittests/tests-cbor/tests-cbor.c @@ -279,6 +279,39 @@ static void test_byte_string(void) } } +static void test_byte_string_no_copy(void) +{ + char buffer[128]; + + { + const char *input = ""; + unsigned char data[] = {0x40}; + unsigned char *out; + size_t size; + TEST_ASSERT(cbor_serialize_byte_string(&stream, input)); + CBOR_CHECK_SERIALIZED(stream, data, sizeof(data)); + TEST_ASSERT(cbor_deserialize_byte_string_no_copy(&stream, 0, &out, &size)); + memcpy(buffer, out, size); + buffer[size] = '\0'; + CBOR_CHECK_DESERIALIZED(input, buffer, EQUAL_STRING); + } + + cbor_clear(&stream); + + { + const char *input = "a"; + unsigned char data[] = {0x41, 0x61}; + unsigned char *out; + size_t size; + TEST_ASSERT(cbor_serialize_byte_string(&stream, input)); + CBOR_CHECK_SERIALIZED(stream, data, sizeof(data)); + TEST_ASSERT(cbor_deserialize_byte_string_no_copy(&stream, 0, &out, &size)); + memcpy(buffer, out, size); + buffer[size] = '\0'; + CBOR_CHECK_DESERIALIZED(input, buffer, EQUAL_STRING); + } +} + static void test_byte_string_invalid(void) { { @@ -318,6 +351,39 @@ static void test_unicode_string(void) } } +static void test_unicode_string_no_copy(void) +{ + char buffer[128]; + + { + const char *input = ""; + unsigned char data[] = {0x60}; + unsigned char *out; + size_t size; + TEST_ASSERT(cbor_serialize_unicode_string(&stream, input)); + CBOR_CHECK_SERIALIZED(stream, data, sizeof(data)); + TEST_ASSERT(cbor_deserialize_unicode_string_no_copy(&stream, 0, &out, &size)); + memcpy(buffer, out, size); + buffer[size] = '\0'; + CBOR_CHECK_DESERIALIZED(input, buffer, EQUAL_STRING); + } + + cbor_clear(&stream); + + { + const char *input = "a"; + unsigned char data[] = {0x61, 0x61}; + unsigned char *out; + size_t size; + TEST_ASSERT(cbor_serialize_unicode_string(&stream, input)); + CBOR_CHECK_SERIALIZED(stream, data, sizeof(data)); + TEST_ASSERT(cbor_deserialize_unicode_string_no_copy(&stream, 0, &out, &size)); + memcpy(buffer, out, size); + buffer[size] = '\0'; + CBOR_CHECK_DESERIALIZED(input, buffer, EQUAL_STRING); + } +} + static void test_unicode_string_invalid(void) { { @@ -761,8 +827,10 @@ TestRef tests_cbor_all(void) new_TestFixture(test_int64_t), new_TestFixture(test_int64_t_invalid), new_TestFixture(test_byte_string), + new_TestFixture(test_byte_string_no_copy), new_TestFixture(test_byte_string_invalid), new_TestFixture(test_unicode_string), + new_TestFixture(test_unicode_string_no_copy), new_TestFixture(test_unicode_string_invalid), new_TestFixture(test_array), new_TestFixture(test_array_indefinite), From f6db56929faf451a9554c2bf556a88f9318e226c Mon Sep 17 00:00:00 2001 From: Lucio Torre Date: Wed, 30 Mar 2016 14:30:15 +0200 Subject: [PATCH 3/6] cbor: check return value for zero --- sys/cbor/cbor.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sys/cbor/cbor.c b/sys/cbor/cbor.c index cc50b4117592..ff264bcdac08 100644 --- a/sys/cbor/cbor.c +++ b/sys/cbor/cbor.c @@ -439,6 +439,10 @@ size_t cbor_deserialize_int(const cbor_stream_t *stream, size_t offset, int *val uint64_t buf; size_t read_bytes = decode_int(stream, offset, &buf); + if (!read_bytes) { + return 0; + } + if (CBOR_TYPE(stream, offset) == CBOR_UINT) { *val = buf; /* resolve as CBOR_UINT */ } From 04fc0b35a75fd2b3124ce178d8edcb8872f189ab Mon Sep 17 00:00:00 2001 From: Lucio Torre Date: Tue, 15 Sep 2015 00:31:58 -0300 Subject: [PATCH 4/6] cbor: introduce cbor_serialize_byte_stringl --- sys/cbor/cbor.c | 10 ++++++++-- sys/include/cbor.h | 10 ++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/sys/cbor/cbor.c b/sys/cbor/cbor.c index ff264bcdac08..1665d79beef7 100644 --- a/sys/cbor/cbor.c +++ b/sys/cbor/cbor.c @@ -404,8 +404,9 @@ static size_t decode_bytes(const cbor_stream_t *s, size_t offset, char *out, siz } /* A zero copy version of decode_bytes. - Will not null termiante input, but tell you the size of what you read. - Great for reading byte strings which could contain nulls inside + Will not null termiante input, but will tell you the size of what you read. + Great for reading byte strings which could contain nulls inside of unknown size + without forced copies. */ static size_t decode_bytes_no_copy(const cbor_stream_t *s, size_t offset, unsigned char **out, size_t *length) { @@ -640,6 +641,11 @@ size_t cbor_serialize_byte_string(cbor_stream_t *stream, const char *val) return encode_bytes(CBOR_BYTES, stream, val, strlen(val)); } +size_t cbor_serialize_byte_stringl(cbor_stream_t *stream, const char *val, size_t length) +{ + return encode_bytes(CBOR_BYTES, stream, val, length); +} + size_t cbor_deserialize_unicode_string(const cbor_stream_t *stream, size_t offset, char *val, size_t length) { diff --git a/sys/include/cbor.h b/sys/include/cbor.h index 31400ce83e0a..f628ac180b55 100644 --- a/sys/include/cbor.h +++ b/sys/include/cbor.h @@ -365,6 +365,16 @@ size_t cbor_deserialize_double(const cbor_stream_t *stream, size_t offset, */ size_t cbor_serialize_byte_string(cbor_stream_t *stream, const char *val); +/** + * @brief Serializes an arbitrary byte string + * + * @param[out] stream The destination stream for serializing the byte stream + * @param[in] val The arbitrary byte string which may include null bytes + * @param[in] length The size of the byte string in bytes + * + * @return Number of bytes written to stream @p stream + */ +size_t cbor_serialize_byte_stringl(cbor_stream_t *stream, const char *val, size_t length); /** * @brief Deserialize bytes from @p stream to @p val From 5f26b7686fb6678bb2c48cd946a16673752f0984 Mon Sep 17 00:00:00 2001 From: Lucio Torre Date: Wed, 30 Mar 2016 14:37:04 +0200 Subject: [PATCH 5/6] cbor: fix typo --- sys/include/cbor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sys/include/cbor.h b/sys/include/cbor.h index f628ac180b55..2b051656f571 100644 --- a/sys/include/cbor.h +++ b/sys/include/cbor.h @@ -397,7 +397,7 @@ size_t cbor_serialize_unicode_string(cbor_stream_t *stream, const char *val); * @param[in] stream The stream to deserialize * @param[in] offset The offset within the stream where to start deserializing * @param[out] val Pointer to a char * - * @param[out] length Pointer tp a size_t to store the size of the string + * @param[out] length Pointer to a size_t to store the size of the string * * @return Number of bytes written into @p val */ From 63bcccf43d014eb133067a950343d928e09d71f4 Mon Sep 17 00:00:00 2001 From: Matias Devenuta Date: Fri, 6 Nov 2015 19:12:12 -0300 Subject: [PATCH 6/6] cbor: Packed struct to bypass unaligned stack This can happen due to cast using buggy GCC on ARMv7 Credit to our shy french pal --- sys/cbor/cbor.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/sys/cbor/cbor.c b/sys/cbor/cbor.c index 1665d79beef7..bbb7c65e74e5 100644 --- a/sys/cbor/cbor.c +++ b/sys/cbor/cbor.c @@ -95,7 +95,20 @@ #define NAN (0.0/0.0) #endif + +/* pack to force aligned access on ARMv7 (buggy GCC) */ +#pragma GCC diagnostic error "-Wcast-align" +typedef struct __attribute__((packed)) { + unsigned char u8; + union { + uint16_t u16; + uint32_t u32; + uint64_t u64; + } u; +} cast_align_u8_t; + #ifndef CBOR_NO_FLOAT + /** * Convert float @p x to network format */ @@ -345,15 +358,15 @@ static size_t decode_int(const cbor_stream_t *s, size_t offset, uint64_t *val) break; case 2: - *val = HTONS(*((uint16_t *)&in[1])); + *val = HTONS(((cast_align_u8_t *)in)->u.u16); break; case 4: - *val = HTONL(*((uint32_t *)&in[1])); + *val = HTONL(((cast_align_u8_t *)in)->u.u32); break; default: - *val = HTONLL(*((uint64_t *)&in[1])); + *val = HTONLL(((cast_align_u8_t *)in)->u.u64); break; } @@ -565,7 +578,7 @@ size_t cbor_deserialize_float(const cbor_stream_t *stream, size_t offset, float unsigned char *data = &stream->data[offset]; if (*data == CBOR_FLOAT32) { - *val = ntohf(*(uint32_t *)(data + 1)); + *val = ntohf(((cast_align_u8_t *)data)->u.u32); return 5; } @@ -594,7 +607,7 @@ size_t cbor_deserialize_double(const cbor_stream_t *stream, size_t offset, doubl if (*data == CBOR_FLOAT64) { CBOR_ENSURE_SIZE_READ(stream, offset + 9); - *val = ntohd(*(uint64_t *)(data + 1)); + *val = ntohd(((cast_align_u8_t *)data)->u.u64); return 9; }