bergzand / NanoCBOR

CBOR library aimed at heavily constrained devices
Creative Commons Zero v1.0 Universal
48 stars 24 forks source link

nanocbor_get_key_tstr returns NANOCBOR_OK even when key is not present #56

Open warraagal opened 3 years ago

warraagal commented 3 years ago

Code for reproducing

#include<stdio.h>
#include<nanocbor/nanocbor.h>

int main(void) {
    char encode_buf[1024];
    nanocbor_encoder_t cbor_encoder;
    nanocbor_encoder_init(&cbor_encoder, encode_buf, 1024);

    nanocbor_fmt_map(&cbor_encoder, 2);
    nanocbor_put_tstr(&cbor_encoder, "key1");
    nanocbor_put_tstr(&cbor_encoder, "value1");
    nanocbor_put_tstr(&cbor_encoder, "key2");
    nanocbor_put_tstr(&cbor_encoder, "value2");

    nanocbor_value_t cbor_decoder;
    nanocbor_value_t cbor_encoded_val;
    nanocbor_value_t parsed_map;
    nanocbor_decoder_init(&cbor_decoder, encode_buf, nanocbor_encoded_len(&cbor_encoder));

    if(nanocbor_enter_map(&cbor_decoder, &parsed_map) == NANOCBOR_OK 
            && nanocbor_get_key_tstr(&parsed_map, "key3", &cbor_encoded_val) == NANOCBOR_OK) {
        printf("key3 found\n");
    } else {
        printf("key3 not found\n");
    }
}

This prints "key3 found". I haven't been able to figure out why this is happening.

bergzand commented 3 years ago

Thanks for the report, this is indeed a bug.

It is caused by the nanocbor_skip call that overwrites the res value returned by the nanocbor_get_key_tstr call. This means that if the final element of the map is skipped with nanocbor_skip and the key is not found, the res value is taken from the nanocbor_skip call and returned.

I'll have a fix out asap

bergzand commented 3 years ago

I have to check if I don't cause a regression here, but I think this patch should fix your issue:

diff --git a/src/decoder.c b/src/decoder.c
index 80e69f2..2fc2282 100644
--- a/src/decoder.c
+++ b/src/decoder.c
@@ -433,8 +433,9 @@ int nanocbor_get_key_tstr(nanocbor_value_t *start, const char *key,
         const uint8_t *s = NULL;
         size_t s_len = 0;

-        if ((res = nanocbor_get_tstr(value, &s, &s_len)) < 0) {
-            break;
+        int call_res = NANOCBOR_OK;
+        if ((call_res = nanocbor_get_tstr(value, &s, &s_len)) < 0) {
+            return call_res;
         }

         if (s_len == len && !strncmp(key, (const char *)s, len)) {
@@ -442,8 +443,8 @@ int nanocbor_get_key_tstr(nanocbor_value_t *start, const char *key,
             break;
         }

-        if ((res = nanocbor_skip(value)) < 0) {
-            break;
+        if ((call_res = nanocbor_skip(value)) < 0) {
+            return call_res;
         }
     }
CromSoldier commented 8 months ago

Hi, I also stubbled upon the same issue. Did you test for regression ?