Skip to content

Commit

Permalink
Bug: crash on non string "alg" (#48)
Browse files Browse the repository at this point in the history 
* release JWS resources in test code after merging #33

* fix crash when "alg" header is set to non-string; see #47

this would affect cjose_jws_sign as well as cjose_jws_import and is a
potential DoS vector for the latter

Signed-off-by: Hans Zandbelt <[email protected]>
  • Loading branch information
@zandbelt @linuxwolf
zandbelt authored and linuxwolf committed May 16, 2017
1 parent ab20e5e commit b5daeb6
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/jws.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ static bool _cjose_jws_validate_hdr(cjose_jws_t *jws, cjose_err *err)
{
// make sure we have an alg header
json_t *alg_obj = json_object_get(jws->hdr, CJOSE_HDR_ALG);
if (NULL == alg_obj)
if ((NULL == alg_obj) || (!json_is_string(alg_obj)))
{
CJOSE_ERROR(err, CJOSE_ERR_INVALID_ARG);
return false;
Expand Down

4 comments on commit b5daeb6

@vtotad
Copy link

@vtotad vtotad commented on b5daeb6 Aug 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In function "cjose_jws_t *cjose_jws_import(const char *cser, size_t cser_len, cjose_err *err)"

// validate the JSON header segment
if (!_cjose_jws_validate_hdr(jws, err))
{
    // make an exception for alg=none so that it will import/parse but not sign/verify
    json_t *alg_obj = json_object_get(jws->hdr, CJOSE_HDR_ALG);
    if (NULL == alg_obj)
    {
        CJOSE_ERROR(err, CJOSE_ERR_INVALID_ARG);
        return NULL;
    }
    const char *alg = json_string_value(alg_obj);
    if ((!alg) || (strcmp(alg, CJOSE_HDR_ALG_NONE) != 0))
    {
        CJOSE_ERROR(err, CJOSE_ERR_INVALID_ARG);
        cjose_jws_release(jws);
        return NULL;
    }
}

My code is getting crash in "cjose_jws_release(jws);" for invalid token.
#0 0x555d2430 in __kernel_vsyscall ()
#1 0x558b3227 in raise () from /lib/libc.so.6
#2 0x558b4a63 in abort () from /lib/libc.so.6
#3 0x558f4fd5 in __libc_message () from /lib/libc.so.6
#4 0x558fdaad in _int_free () from /lib/libc.so.6
#5 0x55f0f550 in jsonp_free (ptr=0x913ad2a) at memory.c:36
#6 0x55f0d186 in hashtable_close (hashtable=hashtable@entry=0x913ad00) at hashtable.c:216
#7 0x55f1226b in json_delete_object (object=0x913acf8) at value.c:77
#8 json_delete (json=0x913acf8) at value.c:936
#9 0x556a23b9 in json_decref (json=0x913acf8) at /usr/include/jansson.h:112
#10 0x556a437c in cjose_jws_release (jws=0x912efa8) at jws.c:705

is same fix required here? or do I need to upgrade json library?

using jansson-2.10.

@zandbelt
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks to me like a different issue; what does your invalid JWT header look like?

@vtotad
Copy link

@vtotad vtotad commented on b5daeb6 Aug 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its because of json-c and jansson library conflict.
json_object_get is returning only main object address and not returning the proper key object address.
My apologies.

@veselov
Copy link
Contributor

@veselov veselov commented on b5daeb6 Aug 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, conflicts between json-c and jansson are a nightmare. I have to rename all json-c symbols to get by.

Please sign in to comment.