zcbor_encode.c: Fix a length bug in zcbor_bstr_end_decode()

The bug caused CBOR-encoded bstrs to lose a byte.

When calculating the maximum length of the bstr, there are certain
payload lengths which cannot be filled by a bstr (incl. header and
payload). One of these lengths is 258, since a 255 byte payload gets a
2-byte header (total 257) and a 256 byte payload gets a 3-byte header
(total 259), i.e. there is no combination that gives 258 bytes. The same
applies to total lengths of 25, 65539-65540, and 4294967300-4294967304

The previous code tried to find the start of the string by extrapolating
from the end of the payload, but this gave the wrong answer for the
cases described above. But also, the remaining_str_len was suboptimally
implemented, so a few more cases were also wrong.

Fix zcbor_bstr_end_encode(), remaining_str_len(), and add tests that
test both functions

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

include/zcbor_common.h

@@ -475,6 +475,18 @@ size_t zcbor_header_len(uint64_t value);
 /** Like @ref zcbor_header_len but for integer of any size <= 8. */
 size_t zcbor_header_len_ptr(const void *const value, size_t value_len);
 
+/** If a string (header + payload) is encoded into the rest of the payload, how long would it be?
+ *
+ *  Note that a string with this length doesn't necessarily fill the rest of the
+ *  payload. For some payload lengths, e.g. 25, it's impossible to encode a
+ *  string of that total length.
+ *
+ *  @param[in] state  The current state.
+ *
+ *  @return  The length of the string (payload, not including header) in bytes.
+ */
+size_t zcbor_remaining_str_len(zcbor_state_t *state);
+
 /** Convert a float16 value to float32.
  *
  *  @param[in] input  The float16 value stored in a uint16_t.

src/zcbor_common.c

@@ -283,6 +283,22 @@ size_t zcbor_header_len_ptr(const void *const value, size_t value_len)
 }
 
 
+size_t zcbor_remaining_str_len(zcbor_state_t *state)
+{
+	size_t max_len = (size_t)state->payload_end - (size_t)state->payload;
+
+	if (max_len == 0) {
+		return 0;
+	}
+
+	size_t max_header_len = zcbor_header_len(max_len);
+	size_t min_header_len = zcbor_header_len(max_len - max_header_len);
+	size_t header_len = zcbor_header_len(max_len - min_header_len);
+
+	return max_len - header_len;
+}
+
+
 int zcbor_entry_function(const uint8_t *payload, size_t payload_len,
 	void *result, size_t *payload_len_out, zcbor_state_t *state, zcbor_decoder_t func,
 	size_t n_states, size_t elem_count)

src/zcbor_encode.c

@@ -224,22 +224,13 @@ static bool str_start_encode(zcbor_state_t *state,
 }
 
 
-static size_t remaining_str_len(zcbor_state_t *state)
-{
-	size_t max_len = (size_t)state->payload_end - (size_t)state->payload;
-	size_t result_len = zcbor_header_len_ptr(&max_len, sizeof(max_len)) - 1;
-
-	return max_len - result_len - 1;
-}
-
-
 bool zcbor_bstr_start_encode(zcbor_state_t *state)
 {
 	if (!zcbor_new_backup(state, 0)) {
 		ZCBOR_FAIL();
 	}
 
-	uint64_t max_len = remaining_str_len(state);
+	uint64_t max_len = zcbor_remaining_str_len(state);
 
 	/* Encode a dummy header */
 	if (!value_encode(state, ZCBOR_MAJOR_TYPE_BSTR, &max_len, sizeof(max_len))) {
@@ -265,7 +256,7 @@ bool zcbor_bstr_end_encode(zcbor_state_t *state, struct zcbor_string *result)
 		ZCBOR_FAIL();
 	}
 
-	result->value = state->payload_end - remaining_str_len(state);
+	result->value = state->payload + zcbor_header_len(zcbor_remaining_str_len(state));
 	result->len = (size_t)payload - (size_t)result->value;
 
 	/* Reencode header of list now that we know the number of elements. */

tests/unit/test1_unit_tests/src/main.c

@@ -1440,4 +1440,105 @@ ZTEST(zcbor_unit_tests, test_zcbor_version)
 }
 
 
+/* Test that CBOR-encoded bstrs are encoded with the correct length. */
+ZTEST(zcbor_unit_tests, test_cbor_encoded_bstr_len)
+{
+	uint8_t payload[50];
+
+#ifdef ZCBOR_VERBOSE
+	for (size_t len = 10; len < 0x108; len++)
+#else
+	for (size_t len = 10; len < 0x10010; len++)
+#endif /* ZCBOR_VERBOSE */
+	{
+		ZCBOR_STATE_E(state_e, 1, payload, len, 0);
+		ZCBOR_STATE_D(state_d, 1, payload, len, 1, 0);
+
+		zassert_true(zcbor_bstr_start_encode(state_e), "len: %d\n", len);
+		zassert_true(zcbor_size_put(state_e, len), "len: %d\n", len);
+		zassert_true(zcbor_bstr_end_encode(state_e, NULL), "len: %d\n", len);
+
+		zassert_true(zcbor_bstr_start_decode(state_d, NULL), "len: %d\n", len);
+		zassert_true(zcbor_size_expect(state_d, len), "len: %d\n", len);
+		zassert_true(zcbor_bstr_end_decode(state_d), "len: %d\n", len);
+	}
+
+#if SIZE_MAX == UINT64_MAX
+	for (size_t len = 0xFFFFFF00; len <= 0x100000100; len++) {
+		ZCBOR_STATE_E(state_e, 1, payload, len, 0);
+		ZCBOR_STATE_D(state_d, 1, payload, len, 1, 0);
+
+		zassert_true(zcbor_bstr_start_encode(state_e), "len: %d\n", len);
+		zassert_true(zcbor_size_put(state_e, len), "len: %d\n", len);
+		zassert_true(zcbor_bstr_end_encode(state_e, NULL), "len: %d\n", len);
+
+		zassert_true(zcbor_bstr_start_decode(state_d, NULL), "len: %d\n", len);
+		zassert_true(zcbor_size_expect(state_d, len), "len: %d\n", len);
+		zassert_true(zcbor_bstr_end_decode(state_d), "len: %d\n", len);
+	}
+#endif /* SIZE_MAX == UINT64_MAX */
+}
+
+
+/* Test zcbor_remaining_str_len().
+ * Some payload lengths are impossible to fill with a properly encoded string,
+ * these have special cases. */
+ZTEST(zcbor_unit_tests, test_remaining_str_len)
+{
+	uint8_t payload[8];
+	ZCBOR_STATE_E(state_e, 1, payload, 0, 0);
+
+	zassert_equal(zcbor_remaining_str_len(state_e), 0, "i: 0\n");
+
+	for (uint64_t i = 1; i <= 0x20000; i++) {
+		size_t offset;
+
+		state_e->payload_end = payload + i;
+
+		switch(i) {
+		case 25:
+		case 0x102:
+			offset = 1;
+			break;
+		case 0x10003:
+		case 0x10004:
+			offset = 2;
+			break;
+		default:
+			offset = 0;
+			break;
+		}
+
+		size_t total_len = zcbor_remaining_str_len(state_e)
+					+ zcbor_header_len(zcbor_remaining_str_len(state_e));
+		zassert_equal(i - offset, total_len, "i: %ld, len: %zu\n", i, total_len);
+	}
+
+#if SIZE_MAX == UINT64_MAX
+	for (uint64_t i = 0xFFFFFF00; i <= 0x100000100; i++) {
+		size_t offset;
+
+		state_e->payload_end = payload + i;
+
+		switch(i) {
+		case 0x100000005:
+		case 0x100000006:
+		case 0x100000007:
+		case 0x100000008:
+			offset = 4;
+			break;
+		default:
+			offset = 0;
+			break;
+		}
+
+		size_t total_len = zcbor_remaining_str_len(state_e)
+					+ zcbor_header_len(zcbor_remaining_str_len(state_e));
+		zassert_equal(i - offset, total_len, "i: %lx, len: %zx\n", i, total_len);
+	}
+#endif
+}
+
+
+
 ZTEST_SUITE(zcbor_unit_tests, NULL, NULL, NULL, NULL, NULL);