Enforce encoding rules for simple values

As per RFC8949 sec 3.3. Simple values less than 24 MUST be encoded in the additional information. Simple values 24 to 31 inclusive are unused. Signed-off-by: Øyvind Rønningstad <[email protected]>
NordicSemiconductor · Aug 15, 2024 · a54e92a · a54e92a
1 parent 5db3b83
commit a54e92a
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 4 deletions.
diff --git a/src/zcbor_decode.c b/src/zcbor_decode.c
@@ -1100,13 +1100,21 @@ bool zcbor_simple_decode(zcbor_state_t *state, uint8_t *result)
 	PRINT_FUNC();
 	INITIAL_CHECKS_WITH_TYPE(ZCBOR_MAJOR_TYPE_SIMPLE);
 
+	uint8_t additional = ZCBOR_ADDITIONAL(*state->payload);
+
 	/* Simple values must be 0-23 (additional is 0-23) or 24-255 (additional is 24).
 	 * Other additional values are not considered simple values. */
-	ZCBOR_ERR_IF(ZCBOR_ADDITIONAL(*state->payload) > 24, ZCBOR_ERR_WRONG_TYPE);
+	ZCBOR_ERR_IF(additional > ZCBOR_VALUE_IS_1_BYTE, ZCBOR_ERR_WRONG_TYPE);
 
 	if (!value_extract(state, result, sizeof(*result), NULL)) {
 		ZCBOR_FAIL();
 	}
+
+	/* Simple values less than 24 MUST be encoded in the additional information.
+	   Simple values 24 to 31 inclusive are unused. Ref: RFC8949 sec 3.3 */
+	if ((additional == ZCBOR_VALUE_IS_1_BYTE) && (*result < 32)) {
+		ERR_RESTORE(ZCBOR_ERR_INVALID_VALUE_ENCODING);
+	}
 	return true;
 }
 

diff --git a/src/zcbor_encode.c b/src/zcbor_encode.c
@@ -445,6 +445,10 @@ bool zcbor_list_map_end_force_encode(zcbor_state_t *state)
 
 bool zcbor_simple_encode(zcbor_state_t *state, uint8_t *input)
 {
+	/* Simple values 24 to 31 inclusive are unused. Ref: RFC8949 sec 3.3 */
+	if ((*input > ZCBOR_VALUE_IN_HEADER) && (*input < 32)) {
+		ZCBOR_ERR(ZCBOR_ERR_INVALID_VALUE_ENCODING);
+	}
 	if (!value_encode(state, ZCBOR_MAJOR_TYPE_SIMPLE, input, sizeof(*input))) {
 		zcbor_log("Error encoding %u (0x%p)\r\n", *input, input);
 		ZCBOR_FAIL();
@@ -455,7 +459,7 @@ bool zcbor_simple_encode(zcbor_state_t *state, uint8_t *input)
 
 bool zcbor_simple_put(zcbor_state_t *state, uint8_t input)
 {
-	return value_encode(state, ZCBOR_MAJOR_TYPE_SIMPLE, &input, sizeof(input));
+	return zcbor_simple_encode(state, &input);
 }
 
 

diff --git a/tests/unit/test1_unit_tests/src/main.c b/tests/unit/test1_unit_tests/src/main.c
@@ -887,22 +887,33 @@ ZTEST(zcbor_unit_tests, test_simple)
 	ZCBOR_STATE_D(state_d, 1, payload1, sizeof(payload1), 16, 0);
 	uint8_t simple1 = 0;
 	uint8_t simple2 = 2;
+	uint8_t simple3 = 0;
 
 	zassert_true(zcbor_simple_encode(state_e, &simple1), NULL);
 	zassert_true(zcbor_simple_put(state_e, 14), NULL);
 	zassert_true(zcbor_simple_put(state_e, 22), NULL);
-	zassert_true(zcbor_simple_put(state_e, 24), NULL);
+	zassert_false(zcbor_simple_put(state_e, 24), NULL);
+	zassert_equal(ZCBOR_ERR_INVALID_VALUE_ENCODING, zcbor_peek_error(state_e), NULL);
+	zassert_true(zcbor_simple_put(state_e, 32), NULL);
 	zassert_true(zcbor_simple_put(state_e, 255), NULL);
+	state_e->payload_mut[0] = 0xF8;
+	state_e->payload_mut[1] = 24; // Invalid value
+	state_e->payload_end += 2;
 
 	zassert_true(zcbor_simple_decode(state_d, &simple2), NULL);
 	zassert_true(zcbor_simple_expect(state_d, 14), NULL);
 	zassert_true(zcbor_nil_expect(state_d, NULL), NULL);
 	zassert_false(zcbor_undefined_expect(state_d, NULL), NULL);
-	zassert_true(zcbor_simple_expect(state_d, 24), NULL);
+	zassert_true(zcbor_simple_expect(state_d, 32), NULL);
 	zassert_false(zcbor_simple_expect(state_d, 254), NULL);
 	zassert_true(zcbor_simple_decode(state_d, &simple1), NULL);
 	zassert_equal(0, simple2, NULL);
 	zassert_equal(255, simple1, NULL);
+
+	zassert_false(zcbor_simple_expect(state_d, 24), NULL);
+	zassert_equal(ZCBOR_ERR_INVALID_VALUE_ENCODING, zcbor_peek_error(state_d), "%s\n", zcbor_error_str(zcbor_peek_error(state_d)));
+	zassert_false(zcbor_simple_decode(state_d, &simple3), NULL);
+	zassert_equal(ZCBOR_ERR_INVALID_VALUE_ENCODING, zcbor_peek_error(state_d), "%s\n", zcbor_error_str(zcbor_peek_error(state_d)));
 }