Errors when decoding large-length bitstrings inside a larger CBOR structure using zcbor_decode as a library

[matthewshaw47]

During the creation of a wrapper library, unexpected behavior was observed when decoding large bitstrings. Mainly:

1. If the end of a bitstring was not aligned with the end of the ZCBOR payload, the decode function would fail with a ZCBOR_ERR_PAYLOAD_NOT_CONSUMED error code. This is an issue if bitstring are within a larger ZCBOR structure... Is this intended behavior and I am using the function(s) incorrectly, or might this be an oversight?

2. When breaking up a bitstring decode with zcbor_bstr_start_decode_fragment, zcbor_bstr_next_fragment, and zcbor_bstr_end_decode, the payload would not properly be consumed, as each "next fragment" call would not advance the payload pointer and instead attempt to increase the size of the payload without reallocation or respect to memory (i.e. would increment payload_end instead of payload in the state object).

I fixed/worked around these issues in my local copy of the library by implementing these changes:

diff --git a/src/zcbor_decode.c b/src/zcbor_decode.c
index 1ced3e0..2dd1a9b 100644
--- a/src/zcbor_decode.c
+++ b/src/zcbor_decode.c
@@ -496,7 +496,6 @@ bool zcbor_bstr_start_decode(zcbor_state_t *state, struct zcbor_string *result)
 
 bool zcbor_bstr_end_decode(zcbor_state_t *state)
 {
-	ZCBOR_ERR_IF(state->payload != state->payload_end, ZCBOR_ERR_PAYLOAD_NOT_CONSUMED);
 
 	if (!zcbor_process_backup(state,
 			ZCBOR_FLAG_RESTORE | ZCBOR_FLAG_CONSUME | ZCBOR_FLAG_KEEP_PAYLOAD,
@@ -528,7 +527,7 @@ static bool start_decode_fragment(zcbor_state_t *state,
 	result->offset = 0;
 	result->total_len = result->fragment.len;
 	partition_fragment(state, result);
-	state->payload_end = state->payload + result->fragment.len;
+	state->payload = state->payload + result->fragment.len;
 
 	return true;
 }
@@ -574,7 +573,7 @@ void zcbor_bstr_next_fragment(zcbor_state_t *state,
 
 	partition_fragment(state, result);
 	zcbor_log("fragment length %zu\r\n", result->fragment.len);
-	state->payload_end = state->payload + result->fragment.len;
+	state->payload = state->payload + result->fragment.len;
 }

[oyvindronningstad]

The zcbor_bstr_start|end_decode* functions are meant for a specific use case, when you have a CBOR structure that is contained within a CBOR bstr (which could be part of a bigger CBOR structure). If you have a bstr you should usually instead use the "regular" zcbor_bstr_expect|decode functions UNLESS your bstr specifically contains CBOR data that you wish to decode on the fly. In this case, you call _start(), then decode the data, then call _end.

If you do have such CBOR bstrs, the assumption is that they do not contain any "extra" bytes, so the ZCBOR_ERR_PAYLOAD_NOT_CONSUMED error occurs if you do not decode all bytes of the bstr payload, to prevent you from missing any data.

Note that the fragment API is being overhauled in #424. Though, unless your CBOR data is fragmented (split into separate buffers in memory), you don't need to use the fragment API.

[matthewshaw47]

So if the bstr is also encoded as CBOR, it would be used in a similar fashion as the zcbor_map_start|end_decode and zcbor_list_start|end_decode functions. That makes sense. In my use case though, I am needing to break the decode into chunks due to memory constraints and the shear size of the bstr I am decoding. To note, there are multiple large bstr's in the CBOR I am decoding.

Looking at #424, it looks like the zcbor_bstr_end_decode will still enforce the end of a fragmented bstr to align with the end of the payload.

[oyvindronningstad]

If you have a fragmented CBOR payload, you use only zcbor_update_state() unless your strings span multiple chunks, in which case you use zcbor_tstr|bstr_decode_fragment() followed by one or more zcbor_next_fragment() if your strings are just strings. If you have CBOR-encoded bstrs, use zcbor_bstr_start_decode_fragment() followed by one or more zcbor_bstr_next_fragment() interspersed with decoding of the strings using other functions. In all cases you must use zcbor_update_state() to switch to a new payload chunk in memory.

the zcbor_bstr_end_decode will still enforce the end of a fragmented bstr to align with the end of the payload.

zcbor_bstr_end_decode() cannot be used with fragmented payloads, you must instead use zcbor_bstr_start_decode_fragment() or, in #424, zcbor_cbor_bstr_fragments_start_decode().

But in all cases when decoding CBOR-encoded bstrs, payload_end will be moved to correspond to the end of the string, so you do not decode outside the string before closing it. It's all handled internally, and does not mean your chunks need to align with the end of the bstr.