zcbor_decode: Fix a bug with enforce_canonical (ZCBOR_CANONICAL) + fl…

…oats

Floats with leading 0s were rejected because the decoder wrongly assumed
they could have been encoded with fewer bytes.

Exclude floats from this check and add some tests

Signed-off-by: Øyvind Rønningstad <[email protected]>

include/zcbor_common.h

@@ -255,6 +255,8 @@ do { \
 #define ZCBOR_NIL_VAL ((uint8_t)22)
 #define ZCBOR_UNDEF_VAL ((uint8_t)23)
 
+#define ZCBOR_IS_FLOAT(header_byte) (((header_byte) >= 0xF9) && ((header_byte) <= 0xFB))
+
 #define ZCBOR_FLAG_RESTORE 1UL ///! Restore from the backup. Overwrite the current state with the state from the backup.
 #define ZCBOR_FLAG_CONSUME 2UL ///! Consume the backup. Remove the backup from the stack of backups.
 #define ZCBOR_FLAG_KEEP_PAYLOAD 4UL ///! Keep the pre-restore payload after restoring.

src/zcbor_decode.c

@@ -148,13 +148,15 @@ static bool value_extract(zcbor_state_t *state,
 	INITIAL_CHECKS();
 	ZCBOR_ERR_IF((state->elem_count == 0), ZCBOR_ERR_LOW_ELEM_COUNT);
 
-	uint8_t additional = ZCBOR_ADDITIONAL(*state->payload);
+	uint8_t header_byte = *state->payload;
+	uint8_t additional = ZCBOR_ADDITIONAL(header_byte);
 	size_t len = 0;
 
 	if ((additional == ZCBOR_VALUE_IS_INDEFINITE_LENGTH) && (indefinite_length_array != NULL)) {
-		/* Indefinite length array. */
+		/* Indefinite length is not allowed in canonical CBOR */
 		ZCBOR_ERR_IF(ZCBOR_ENFORCE_CANONICAL(state),
 			ZCBOR_ERR_INVALID_VALUE_ENCODING);
+
 		*indefinite_length_array = true;
 	} else {
 		len = additional_len(additional);
@@ -172,7 +174,9 @@ static bool value_extract(zcbor_state_t *state,
 		} else {
 			endian_copy(result_offs, state->payload + 1, len);
 
-			if (ZCBOR_ENFORCE_CANONICAL(state)) {
+			/* Check whether value could have been encoded shorter.
+			   Only check when enforcing canonical CBOR, and never check floats. */
+			if (ZCBOR_ENFORCE_CANONICAL(state) && !ZCBOR_IS_FLOAT(header_byte)) {
 				ZCBOR_ERR_IF((zcbor_header_len_ptr(result, result_len) != (len + 1)),
 					ZCBOR_ERR_INVALID_VALUE_ENCODING);
 			}

tests/decode/test5_corner_cases/src/main.c

@@ -1698,13 +1698,21 @@ ZTEST(cbor_decode_test5, test_prelude)
 ZTEST(cbor_decode_test5, test_cbor_bstr)
 {
 
+// Length under 23/24
 #ifdef TEST_INDEFINITE_LENGTH_ARRAYS
-#define CBOR_BSTR_LEN(len) (len + 1)
+#define CBOR_BSTR_LEN1(len) (0x40 + len + 1)
 #else
-#define CBOR_BSTR_LEN(len) len
+#define CBOR_BSTR_LEN1(len) (0x40 + len)
+#endif
+
+// Length between 23/24 and 254/255
+#ifdef TEST_INDEFINITE_LENGTH_ARRAYS
+#define CBOR_BSTR_LEN2(len) 0x58, (len + 1)
+#else
+#define CBOR_BSTR_LEN2(len) 0x58, len
 #endif
 	uint8_t cbor_bstr_payload1[] = {
-		0x58, CBOR_BSTR_LEN(33),
+		CBOR_BSTR_LEN2(33),
 			LIST(4),
 				0x46, 0x65, 'H', 'e', 'l', 'l', 'o',
 				0x49, 0xFB, 0x40, 0x9, 0x21, 0xca, 0xc0, 0x83, 0x12, 0x6f /* 3.1415 */,
@@ -1714,7 +1722,7 @@ ZTEST(cbor_decode_test5, test_cbor_bstr)
 	};
 
 	uint8_t cbor_bstr_payload2_inv[] = {
-		0x58, CBOR_BSTR_LEN(33),
+		CBOR_BSTR_LEN2(33),
 			LIST(4),
 				0x46, 0x65, 'Y', 'e', 'l', 'l', 'o',
 				0x49, 0xFB, 0x40, 0x9, 0x21, 0xca, 0xc0, 0x83, 0x12, 0x6f /* 3.1415 */,
@@ -1724,7 +1732,7 @@ ZTEST(cbor_decode_test5, test_cbor_bstr)
 	};
 
 	uint8_t cbor_bstr_payload3_inv[] = {
-		0x58, CBOR_BSTR_LEN(33),
+		CBOR_BSTR_LEN2(33),
 			LIST(4),
 				0x46, 0x65, 'H', 'e', 'l', 'l', 'o',
 				0x49, 0xFB, 0x40, 0x9, 0x21, 0xca, 0, 0, 0, 0 /* 3.1415 */,
@@ -1734,23 +1742,23 @@ ZTEST(cbor_decode_test5, test_cbor_bstr)
 	};
 
 	uint8_t cbor_bstr_payload4_inv[] = {
-		0x58, CBOR_BSTR_LEN(18),
+		CBOR_BSTR_LEN1(18),
 			LIST(3),
 				0x46, 0x65, 'H', 'e', 'l', 'l', 'o',
 				0x49, 0xFB, 0x40, 0x9, 0x21, 0xca, 0xc0, 0x83, 0x12, 0x6f /* 3.1415 */,
 			END
 	};
 
 	uint8_t cbor_bstr_payload5_inv[] = {
-		0x58, CBOR_BSTR_LEN(18),
+		CBOR_BSTR_LEN1(18),
 			LIST(2),
 				0x46, 0x65, 'H', 'e', 'l', 'l', 'o',
 				0x49, 0xFB, 0x40, 0x9, 0x21, 0xca, 0xc0, 0x83, 0x12, 0x6f /* 3.1415 */,
 			END
 	};
 
 	uint8_t cbor_bstr_payload6_inv[] = {
-		0x58, CBOR_BSTR_LEN(33),
+		CBOR_BSTR_LEN2(33),
 			LIST(4),
 				0x46, 0x65, 'H', 'e', 'l', 'l', 'o',
 				0x49, 0xFB, 0x40, 0x9, 0x21, 0xca, 0xc0, 0x83, 0x12, 0x6f /* 3.1415 */,
@@ -1773,10 +1781,10 @@ ZTEST(cbor_decode_test5, test_cbor_bstr)
 	zassert_equal(ZCBOR_ERR_PAYLOAD_NOT_CONSUMED, cbor_decode_CBORBstr(cbor_bstr_payload3_inv, sizeof(cbor_bstr_payload3_inv), &result, &num_decode), NULL);
 
 	res = cbor_decode_CBORBstr(cbor_bstr_payload4_inv, sizeof(cbor_bstr_payload4_inv), &result, &num_decode);
-	zassert_equal(ARR_ERR4, res, "%d\r\n", res);
+	zassert_equal(ARR_ERR4, res, "%s\r\n", zcbor_error_str(res));
 
 	res = cbor_decode_CBORBstr(cbor_bstr_payload5_inv, sizeof(cbor_bstr_payload5_inv), &result, &num_decode);
-	zassert_equal(ARR_ERR4, res, "%d\r\n", res);
+	zassert_equal(ARR_ERR4, res, "%s\r\n", zcbor_error_str(res));
 
 	res = cbor_decode_CBORBstr(cbor_bstr_payload6_inv, sizeof(cbor_bstr_payload6_inv), &result, &num_decode);
 	zassert_equal(ZCBOR_ERR_PAYLOAD_NOT_CONSUMED, res, "%d\r\n", res);

tests/decode/test5_corner_cases/testcase.yaml

@@ -2,6 +2,7 @@ tests:
   zcbor.decode.test5_corner_cases:
     platform_allow: native_posix native_posix/native/64 mps2/an521/cpu0 qemu_malta/qemu_malta/be
     tags: zcbor decode test5
+    extra_args: CANONICAL=1
   zcbor.decode.test5_corner_cases.indefinite_length_arrays:
     platform_allow: native_posix native_posix/native/64 mps2/an521/cpu0 qemu_malta/qemu_malta/be
     tags: zcbor decode test5 indefinite

tests/unit/test1_unit_tests/src/main.c

@@ -8,6 +8,8 @@
 #include "zcbor_decode.h"
 #include "zcbor_encode.h"
 #include "zcbor_print.h"
+#include <math.h>
+
 
 ZTEST(zcbor_unit_tests, test_int64)
 {
@@ -1539,6 +1541,60 @@ ZTEST(zcbor_unit_tests, test_remaining_str_len)
 #endif
 }
 
+/* Test that CBOR-encoded tstrs are encoded with the correct length, even if the first part of the
+   number is 0s. */
+ZTEST(zcbor_unit_tests, test_float_len)
+{
+	uint8_t payload[50];
+	ZCBOR_STATE_E(state_e, 1, payload, sizeof(payload), 0);
+	ZCBOR_STATE_D(state_d, 1, payload, sizeof(payload), 20, 0);
+
+	zassert_true(zcbor_float16_put(state_e, 0.0));
+	zassert_true(zcbor_float16_expect(state_d, 0.0));
+	zassert_true(zcbor_float32_put(state_e, 0.0));
+	zassert_true(zcbor_float32_expect(state_d, 0.0));
+	zassert_true(zcbor_float64_put(state_e, 0.0));
+	zassert_true(zcbor_float64_expect(state_d, 0.0));
+
+	zassert_true(zcbor_float16_put(state_e, powf(2, -17)));
+	zassert_true(zcbor_float16_expect(state_d, powf(2, -17)));
+	zassert_true(zcbor_float32_put(state_e, 0.000000000000000000000000000000000000000001f));
+	zassert_true(zcbor_float32_expect(state_d, 0.000000000000000000000000000000000000000001f));
+	zassert_true(zcbor_float64_put(state_e, pow(10, -315)));
+	zassert_true(zcbor_float64_expect(state_d, pow(10, -315)));
+}
+
+ZTEST(zcbor_unit_tests, test_simple_value_len)
+{
+#ifndef ZCBOR_CANONICAL
+	printf("Skip on non-canonical builds.\n");
+#else
+	uint8_t payload[] = {0xe1, 0xf4, 0xf5, 0xf6, 0xf7};
+
+	/* Simple values under 24 must be encoded as single bytes. */
+	uint8_t payload_inv[] = {
+		0xf8, 1,
+		0xf8, ZCBOR_BOOL_TO_SIMPLE,
+		0xf8, ZCBOR_BOOL_TO_SIMPLE + 1,
+		0xf8, ZCBOR_NIL_VAL,
+		0xf8, ZCBOR_UNDEF_VAL
+	};
+	ZCBOR_STATE_D(state_d, 1, payload, sizeof(payload), 20, 0);
+	ZCBOR_STATE_D(state_d_inv, 1, payload_inv, sizeof(payload_inv), 20, 0);
+
+	zassert_true(zcbor_simple_expect(state_d, 1));
+	zassert_true(zcbor_bool_expect(state_d, false));
+	zassert_true(zcbor_bool_expect(state_d, true));
+	zassert_true(zcbor_nil_expect(state_d, NULL));
+	zassert_true(zcbor_undefined_expect(state_d, NULL));
+
+	zassert_false(zcbor_simple_expect(state_d_inv, 1));
+	zassert_false(zcbor_bool_expect(state_d_inv, false));
+	zassert_false(zcbor_bool_expect(state_d_inv, true));
+	zassert_false(zcbor_nil_expect(state_d_inv, NULL));
+	zassert_false(zcbor_undefined_expect(state_d_inv, NULL));
+#endif
+}
 
 
 ZTEST_SUITE(zcbor_unit_tests, NULL, NULL, NULL, NULL, NULL);