In _decode function, memset the allocated memory to 0 before initializing it

[mysticlife1111]

In File cjose/src/base64.c

In function
static inline bool _decode(const char *input, size_t inlen, uint8_t **output, size_t *outlen, bool url, cjose_err *err)

Line 78:
uint8_t *buffer = cjose_get_alloc()(sizeof(uint8_t) * rlen);

Fix:
memset(buffer, 0, sizeof(uint8_t) * rlen);

Symptoms:
I am using function

jws = cjose_jws_import(access_token, strlen(access_token), err);
to get the jws object and then to get the plaintext version i am using
cjose_jws_get_plaintext(jws, &plaintext, &plaintext_len, err)

However, when i see the plaintext version, it has extra garbage characters in the end after the termination of the JSON

i am using the JSON parse to get the values, C function is
e = cl_json_parse(a_token, &json_top);

It is throwing error because of the corrupted plaintext field (extra garbage character), as i can't change the source code of cjose, workaround is parse from the end till i get the closing } brace to get the final length.

One weird thing is "first time when i decode the string then the JSON plaintext looks ok, its subsequent time when the garbage characters shows up" not sure if it is due to the alloc function.

Please memset the buffer to 0 before using it.

[balthorium]

@mysticlife1111 the outputs of both cjose_jws_get_plaintext() and _decode() functions are byte buffers of explicit length, not zero-terminated character strings. You may want to check your code for a buffer over-read error, which could happen if you are treating the plaintext as a zero-terminated string when the producer of the JWS did not include a terminating zero in the original payload.

If you're certain this is not the case, though, it would be great if you could share a small runnable example of code (with imported JWS) that demonstrates the problem.

[mysticlife1111]

Hi,

cjose_jws_t *jws = NULL;
size_t plaintext_len = 0;
uint8_t *plaintext = NULL;

I am using this function

jws = cjose_jws_import(access_token, strlen(access_token), err);

/* Get the Plaintext information */
if (cjose_jws_get_plaintext(jws, &plaintext, &plaintext_len, err)) {

So i am expecting that plaintext_len will give me the right length of the plaintext,

Here is the decoded JWT in plaintext
{"jti":"e9fd6b08-5804-4e29-8787-1d241dc5cb40","iss":"eCDM AAA Service","iat":1523404709,"exp":1600000020,"ext":{"system":{"role":["System Admin"]},"audit":{"uid":"00000000-0000-4000-a000-000000000000"}},"sub":"[email protected]","authorization-token-bitmap":{"username":"admin","authenticated":true,"id":"00000000-0000-4000-a000-000000000000","userType":"LOCAL","timestamp":0,"creationTime":0,"tenantScope":"/00000000-0000-4000-b000-000000000000/00000000-0000-4000-a000-000000000000","authorities":[{"tenants":["00000000-0000-4000-a000-000000000000","00000000-0000-4000-b000-000000000000"],"privileges":["68719476735"],"roles":["27815934-7826-4e9c-b53d-99d5ac045ffc"]}]},"rti":"f20a4492-530f-4aaa-9a77-0428c9a954b9"}È:F:^U^?

È:F:^U^? are the extra characters, this causes error when i use the JSON parser as it complains JSON string is wrong.

I have to get the actual plaintext len as follows
/* Remove the trailing garbage from the plaintext before JSON parsing */
while (plaintext_len != 0) {
if (plaintext[plaintext_len-1] == '}') {
break;
}
plaintext_len = plaintext_len - 1;
}

I am releasing the memory

if (jws) {
cjose_jws_release(jws);
jws = NULL;
plaintext = NULL;
plaintext_len = 0;
}

Seems like cjose_jws_release(jws); will release the plaintext memory as well.

[zandbelt]

Use strlen(access_token)+1 to include the trailing \0 in the decoded result.

[mysticlife1111]

I tried using strlen(access_token) + 1 but now i am getting error in the function cjose_jws_import

[linuxwolf]

If jws_get_plaintext() is not setting plaintext_len to the right length, that is a bug we need to fix. Zeroing out all the memory, while a good idea, shouldn't be solving the problem. Despite the name, plaintext is binary data, and NULL-terminating doesn't make sense in that case.

The referenced cl_json_parse doesn't take a length, so I assume you are modifying plaintext to add a NULL-terminated value? I'm not sure we've gotten enough context for how you're working with things.

Also, yes cjose_jws_release() does release the plaintext memory, as the ownership belongs to the cjose_jws_t. You'll need to copy it to another buffer you own the memory of if you need to keep it around.

[mysticlife1111]

This is the list of functions i am using

1. jws = cjose_jws_import(access_token, strlen(access_token), err);
2. cjose_jws_get_plaintext(jws, &plaintext, &plaintext_len, err)
3. e = cl_json_parse(a_token, &json_top);

In the second step, plaintext_len gives length including garbage in the end and the step 3 fails. So had to get the right length in plaintext_len removing the garbage characters in the end.

strlen(access_token) + 1 doesn't work!

[linuxwolf]

@mysticlife1111 thank you for the context. I'll admit I'm puzzled why plaintext_len is invalid for you but the tests pass. If you have a chance, please try make test and let us know if that succeeds or fails. In the meantime, I'm taking another look at our existing tests to see where gaps might exist.

I'm also curious how your a_token is initialized. I'm assuming there is code between your steps one and two that takes the bytes pointed to by plaintext are then pointed to by a_token, with NULL termination.

[mysticlife1111]

Hello,

yes

First getting the jws
Step 1: jws = cjose_jws_import(access_token, strlen(access_token), err);

after getting the plaintext

Step 2: cjose_jws_get_plaintext(jws, &plaintext, &plaintext_len, err);

Step 3: /* Remove the trailing garbage from the plaintext before JSON parsing */
while (plaintext_len != 0) {
if (plaintext[plaintext_len-1] == '}') {
break;
}
plaintext_len = plaintext_len - 1;
}

Step 4: a_token = calloc(1, plaintext_len + 1);
panic_if_null(a_token);

Step 5: snprintf(a_token, plaintext_len + 1, "%s", plaintext);

Step 6: /* Now parse the plaintext JSON to get JSON top object */
e = cl_json_parse(a_token, &json_top);
if (e != DD_OK) {
// error
}

Regards!