diff --git a/CHANGELOG.md b/CHANGELOG.md index b6e6ddca..c25a05d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Template: Next --------------------- +- [Fix a regression in `cbor_serialize_alloc` that caused serialization of zero-length strings and bytestrings or byte/strings with zero-length chunks to fail](https://github.com/PJK/libcbor/pull/260) (discovered by [martelletto](https://github.com/martelletto)) 0.10.0 (2022-12-29) --------------------- diff --git a/src/cbor/serialization.c b/src/cbor/serialization.c index e3bb7df5..40f4c531 100644 --- a/src/cbor/serialization.c +++ b/src/cbor/serialization.c @@ -74,11 +74,16 @@ size_t cbor_serialized_size(const cbor_item_t *item) { case CBOR_INT_64: return 9; } + // Note: We do not _cbor_safe_signaling_add zero-length definite strings, + // they would cause zeroes to propagate. All other items are at least one + // byte. case CBOR_TYPE_BYTESTRING: { if (cbor_bytestring_is_definite(item)) { - return _cbor_safe_signaling_add( - _cbor_encoded_header_size(cbor_bytestring_length(item)), - cbor_bytestring_length(item)); + size_t header_size = + _cbor_encoded_header_size(cbor_bytestring_length(item)); + if (cbor_bytestring_length(item) == 0) return header_size; + return _cbor_safe_signaling_add(header_size, + cbor_bytestring_length(item)); } size_t indef_bytestring_size = 2; // Leading byte + break cbor_item_t **chunks = cbor_bytestring_chunks_handle(item); @@ -90,9 +95,10 @@ size_t cbor_serialized_size(const cbor_item_t *item) { } case CBOR_TYPE_STRING: { if (cbor_string_is_definite(item)) { - return _cbor_safe_signaling_add( - _cbor_encoded_header_size(cbor_string_length(item)), - cbor_string_length(item)); + size_t header_size = + _cbor_encoded_header_size(cbor_string_length(item)); + if (cbor_string_length(item) == 0) return header_size; + return _cbor_safe_signaling_add(header_size, cbor_string_length(item)); } size_t indef_string_size = 2; // Leading byte + break cbor_item_t **chunks = cbor_string_chunks_handle(item); diff --git a/test/cbor_serialize_test.c b/test/cbor_serialize_test.c index a0a3691a..b42744ab 100644 --- a/test/cbor_serialize_test.c +++ b/test/cbor_serialize_test.c @@ -505,6 +505,7 @@ static void test_auto_serialize_too_large(void **_CBOR_UNUSED(_state)) { // Pretend the chunk is huge chunk->metadata.string_metadata.length = SIZE_MAX; + assert_true(SIZE_MAX + 2 == 1); assert_int_equal(cbor_serialized_size(item), 0); unsigned char *output; size_t output_size; @@ -531,6 +532,104 @@ static void test_auto_serialize_alloc_fail(void **_CBOR_UNUSED(_state)) { cbor_decref(&item); } +static void test_auto_serialize_zero_len_bytestring( + void **_CBOR_UNUSED(_state)) { + cbor_item_t *item = cbor_build_bytestring((cbor_data) "", 0); + + unsigned char *output; + assert_int_equal(cbor_serialize_alloc(item, &output, NULL), 1); + assert_memory_equal(output, ((unsigned char[]){0x40}), 1); + assert_int_equal(cbor_serialized_size(item), 1); + cbor_decref(&item); + _cbor_free(output); +} + +static void test_auto_serialize_zero_len_string(void **_CBOR_UNUSED(_state)) { + cbor_item_t *item = cbor_build_string(""); + + unsigned char *output; + assert_int_equal(cbor_serialize_alloc(item, &output, NULL), 1); + assert_memory_equal(output, ((unsigned char[]){0x60}), 1); + assert_int_equal(cbor_serialized_size(item), 1); + cbor_decref(&item); + _cbor_free(output); +} + +static void test_auto_serialize_zero_len_bytestring_chunk( + void **_CBOR_UNUSED(_state)) { + cbor_item_t *item = cbor_new_indefinite_bytestring(); + + assert_true(cbor_bytestring_add_chunk( + item, cbor_move(cbor_build_bytestring((cbor_data) "", 0)))); + + unsigned char *output; + assert_int_equal(cbor_serialize_alloc(item, &output, NULL), 3); + assert_memory_equal(output, ((unsigned char[]){0x5f, 0x40, 0xff}), 3); + assert_int_equal(cbor_serialized_size(item), 3); + cbor_decref(&item); + _cbor_free(output); +} + +static void test_auto_serialize_zero_len_string_chunk( + void **_CBOR_UNUSED(_state)) { + cbor_item_t *item = cbor_new_indefinite_string(); + + assert_true(cbor_string_add_chunk(item, cbor_move(cbor_build_string("")))); + + unsigned char *output; + assert_int_equal(cbor_serialize_alloc(item, &output, NULL), 3); + assert_memory_equal(output, ((unsigned char[]){0x7f, 0x60, 0xff}), 3); + assert_int_equal(cbor_serialized_size(item), 3); + cbor_decref(&item); + _cbor_free(output); +} + +static void test_auto_serialize_zero_len_array(void **_CBOR_UNUSED(_state)) { + cbor_item_t *item = cbor_new_definite_array(0); + + unsigned char *output; + assert_int_equal(cbor_serialize_alloc(item, &output, NULL), 1); + assert_memory_equal(output, ((unsigned char[]){0x80}), 1); + assert_int_equal(cbor_serialized_size(item), 1); + cbor_decref(&item); + _cbor_free(output); +} + +static void test_auto_serialize_zero_len_indef_array( + void **_CBOR_UNUSED(_state)) { + cbor_item_t *item = cbor_new_indefinite_array(); + + unsigned char *output; + assert_int_equal(cbor_serialize_alloc(item, &output, NULL), 2); + assert_memory_equal(output, ((unsigned char[]){0x9f, 0xff}), 2); + assert_int_equal(cbor_serialized_size(item), 2); + cbor_decref(&item); + _cbor_free(output); +} + +static void test_auto_serialize_zero_len_map(void **_CBOR_UNUSED(_state)) { + cbor_item_t *item = cbor_new_definite_map(0); + + unsigned char *output; + assert_int_equal(cbor_serialize_alloc(item, &output, NULL), 1); + assert_memory_equal(output, ((unsigned char[]){0xa0}), 1); + assert_int_equal(cbor_serialized_size(item), 1); + cbor_decref(&item); + _cbor_free(output); +} + +static void test_auto_serialize_zero_len_indef_map( + void **_CBOR_UNUSED(_state)) { + cbor_item_t *item = cbor_new_indefinite_map(); + + unsigned char *output; + assert_int_equal(cbor_serialize_alloc(item, &output, NULL), 2); + assert_memory_equal(output, ((unsigned char[]){0xbf, 0xff}), 2); + assert_int_equal(cbor_serialized_size(item), 2); + cbor_decref(&item); + _cbor_free(output); +} + int main(void) { const struct CMUnitTest tests[] = { cmocka_unit_test(test_serialize_uint8_embed), @@ -570,6 +669,14 @@ int main(void) { cmocka_unit_test(test_auto_serialize_no_size), cmocka_unit_test(test_auto_serialize_too_large), cmocka_unit_test(test_auto_serialize_alloc_fail), + cmocka_unit_test(test_auto_serialize_zero_len_bytestring), + cmocka_unit_test(test_auto_serialize_zero_len_string), + cmocka_unit_test(test_auto_serialize_zero_len_bytestring_chunk), + cmocka_unit_test(test_auto_serialize_zero_len_string_chunk), + cmocka_unit_test(test_auto_serialize_zero_len_array), + cmocka_unit_test(test_auto_serialize_zero_len_indef_array), + cmocka_unit_test(test_auto_serialize_zero_len_map), + cmocka_unit_test(test_auto_serialize_zero_len_indef_map), }; return cmocka_run_group_tests(tests, NULL, NULL); }