From 83beeae39bf48f3957c80bf0fd03c221bce15bc1 Mon Sep 17 00:00:00 2001 From: Martin Schanzenbach Date: Fri, 26 Mar 2021 11:45:51 +0100 Subject: -fix some coverity issues wrt reclaim --- src/reclaim/gnunet-service-reclaim.c | 5 +- src/reclaim/gnunet-service-reclaim_tickets.c | 15 ++- src/reclaim/oidc_helper.c | 14 ++- src/reclaim/plugin_rest_openid_connect.c | 50 +++++++--- src/reclaim/plugin_rest_reclaim.c | 131 +++++++++++++++------------ src/reclaim/reclaim_api.c | 20 ++-- 6 files changed, 141 insertions(+), 94 deletions(-) (limited to 'src/reclaim') diff --git a/src/reclaim/gnunet-service-reclaim.c b/src/reclaim/gnunet-service-reclaim.c index 913b667b7..04c12735b 100644 --- a/src/reclaim/gnunet-service-reclaim.c +++ b/src/reclaim/gnunet-service-reclaim.c @@ -731,9 +731,12 @@ static int check_issue_ticket_message (void *cls, const struct IssueTicketMessage *im) { uint16_t size; + size_t attrs_len; size = ntohs (im->header.size); - if (size <= sizeof(struct IssueTicketMessage)) + attrs_len = ntohs (im->attr_len); + + if (attrs_len > size - sizeof(struct IssueTicketMessage)) { GNUNET_break (0); return GNUNET_SYSERR; diff --git a/src/reclaim/gnunet-service-reclaim_tickets.c b/src/reclaim/gnunet-service-reclaim_tickets.c index ef2303bd7..48b3fe214 100644 --- a/src/reclaim/gnunet-service-reclaim_tickets.c +++ b/src/reclaim/gnunet-service-reclaim_tickets.c @@ -690,7 +690,10 @@ rvk_move_attr_cb (void *cls, return; } GNUNET_RECLAIM_id_generate (&rvk->move_attr->new_id); - new_label = NULL; + new_label = + GNUNET_STRINGS_data_to_string_alloc (&rvk->move_attr->new_id, + sizeof (rvk->move_attr->new_id)); + attr_data = NULL; // new_rd = *rd; for (int i = 0; i < rd_count; i++) @@ -714,9 +717,6 @@ rvk_move_attr_cb (void *cls, new_rd[i].record_type = rd[i].record_type; new_rd[i].flags = rd[i].flags; new_rd[i].expiration_time = rd[i].expiration_time; - new_label = - GNUNET_STRINGS_data_to_string_alloc (&rvk->move_attr->new_id, - sizeof (rvk->move_attr->new_id)); GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Adding attribute %s\n", new_label); GNUNET_free (claim); } @@ -738,9 +738,6 @@ rvk_move_attr_cb (void *cls, new_rd[i].record_type = rd[i].record_type; new_rd[i].flags = rd[i].flags; new_rd[i].expiration_time = rd[i].expiration_time; - new_label = - GNUNET_STRINGS_data_to_string_alloc (&rvk->move_attr->new_id, - sizeof (rvk->move_attr->new_id)); GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Adding credential %s\n", new_label); GNUNET_free (credential); @@ -1400,7 +1397,7 @@ issue_ticket (struct TicketIssueHandle *ih) attrs_record, &store_ticket_issue_cont, ih); - for (j = 0; j > i; j++) + for (j = 0; j < i; j++) { if (attrs_record[j].record_type != GNUNET_GNSRECORD_TYPE_RECLAIM_PRESENTATION) @@ -1585,7 +1582,7 @@ filter_tickets_cb (void *cls, cleanup_issue_handle (tih); return; } - + GNUNET_RECLAIM_presentation_list_destroy (ticket_presentations); // ticket not found in current record, checking next record set GNUNET_NAMESTORE_zone_iterator_next (tih->ns_it, 1); } diff --git a/src/reclaim/oidc_helper.c b/src/reclaim/oidc_helper.c index bd3a8ee05..7b24ee598 100644 --- a/src/reclaim/oidc_helper.c +++ b/src/reclaim/oidc_helper.c @@ -839,11 +839,17 @@ int OIDC_access_token_parse (const char *token, struct GNUNET_RECLAIM_Ticket **ticket) { - if (sizeof (struct GNUNET_RECLAIM_Ticket) != - GNUNET_STRINGS_base64_decode (token, - strlen (token), - (void**) ticket)) + size_t sret; + char *decoded; + sret = GNUNET_STRINGS_base64_decode (token, + strlen (token), + (void**) &decoded); + if (sizeof (struct GNUNET_RECLAIM_Ticket) != sret) + { + GNUNET_free (decoded); return GNUNET_SYSERR; + } + *ticket = (struct GNUNET_RECLAIM_Ticket *) decoded; return GNUNET_OK; } diff --git a/src/reclaim/plugin_rest_openid_connect.c b/src/reclaim/plugin_rest_openid_connect.c index 0ee61755b..c6259d745 100644 --- a/src/reclaim/plugin_rest_openid_connect.c +++ b/src/reclaim/plugin_rest_openid_connect.c @@ -686,7 +686,10 @@ do_userinfo_error (void *cls) handle->emsg, (NULL != handle->edesc) ? handle->edesc : ""); resp = GNUNET_REST_create_response (""); - MHD_add_response_header (resp, MHD_HTTP_HEADER_WWW_AUTHENTICATE, "Bearer"); + GNUNET_assert (MHD_NO != + MHD_add_response_header (resp, + MHD_HTTP_HEADER_WWW_AUTHENTICATE, + "Bearer")); handle->proc (handle->proc_cls, resp, handle->response_code); cleanup_handle (handle); GNUNET_free (error); @@ -713,7 +716,8 @@ do_redirect_error (void *cls) (NULL != handle->oidc->state) ? "&state=" : "", (NULL != handle->oidc->state) ? handle->oidc->state : ""); resp = GNUNET_REST_create_response (""); - MHD_add_response_header (resp, "Location", redirect); + GNUNET_assert (MHD_NO != MHD_add_response_header (resp, + "Location", redirect)); handle->proc (handle->proc_cls, resp, MHD_HTTP_FOUND); cleanup_handle (handle); GNUNET_free (redirect); @@ -1022,7 +1026,8 @@ oidc_ticket_issue_cb (void *cls, handle->oidc->state); } resp = GNUNET_REST_create_response (""); - MHD_add_response_header (resp, "Location", redirect_uri); + GNUNET_assert (MHD_NO != MHD_add_response_header (resp, + "Location", redirect_uri)); handle->proc (handle->proc_cls, resp, MHD_HTTP_FOUND); cleanup_handle (handle); GNUNET_free (redirect_uri); @@ -1381,7 +1386,8 @@ build_redirect (void *cls) handle->oidc->state); } resp = GNUNET_REST_create_response (""); - MHD_add_response_header (resp, "Location", redirect_uri); + GNUNET_assert (MHD_NO != MHD_add_response_header (resp, + "Location", redirect_uri)); handle->proc (handle->proc_cls, resp, MHD_HTTP_FOUND); cleanup_handle (handle); GNUNET_free (redirect_uri); @@ -1764,8 +1770,12 @@ login_cont (struct GNUNET_REST_RequestHandle *con_handle, "%s;Max-Age=%d", cookie, OIDC_COOKIE_EXPIRATION); - MHD_add_response_header (resp, "Set-Cookie", header_val); - MHD_add_response_header (resp, "Access-Control-Allow-Methods", "POST"); + GNUNET_assert (MHD_NO != MHD_add_response_header (resp, + "Set-Cookie", header_val)); + GNUNET_assert (MHD_NO != + MHD_add_response_header (resp, + "Access-Control-Allow-Methods", + "POST")); GNUNET_CRYPTO_hash (cookie, strlen (cookie), &cache_key); if (0 != strcmp (json_string_value (identity), "Denied")) @@ -1880,7 +1890,8 @@ parse_credentials_post_body (struct RequestHandle *handle, } pass = GNUNET_CONTAINER_multihashmap_get (handle->rest_handle->url_param_map, &cache_key); - if (NULL == pass) { + if (NULL == pass) + { GNUNET_free (*client_id); *client_id = NULL; return GNUNET_SYSERR; @@ -2134,6 +2145,8 @@ token_endpoint (struct GNUNET_REST_RequestHandle *con_handle, GNUNET_free (code); if (NULL != nonce) GNUNET_free (nonce); + GNUNET_RECLAIM_attribute_list_destroy (cl); + GNUNET_RECLAIM_presentation_list_destroy (pl); GNUNET_SCHEDULER_add_now (&do_error, handle); return; } @@ -2149,6 +2162,8 @@ token_endpoint (struct GNUNET_REST_RequestHandle *con_handle, handle->edesc = GNUNET_strdup ("No signing secret configured!"); handle->response_code = MHD_HTTP_INTERNAL_SERVER_ERROR; GNUNET_free (code); + GNUNET_RECLAIM_attribute_list_destroy (cl); + GNUNET_RECLAIM_presentation_list_destroy (pl); if (NULL != nonce) GNUNET_free (nonce); GNUNET_SCHEDULER_add_now (&do_error, handle); @@ -2191,9 +2206,14 @@ token_endpoint (struct GNUNET_REST_RequestHandle *con_handle, &json_response); resp = GNUNET_REST_create_response (json_response); - MHD_add_response_header (resp, "Cache-Control", "no-store"); - MHD_add_response_header (resp, "Pragma", "no-cache"); - MHD_add_response_header (resp, "Content-Type", "application/json"); + GNUNET_assert (MHD_NO != MHD_add_response_header (resp, + "Cache-Control", + "no-store")); + GNUNET_assert (MHD_NO != MHD_add_response_header (resp, + "Pragma", "no-cache")); + GNUNET_assert (MHD_NO != MHD_add_response_header (resp, + "Content-Type", + "application/json")); handle->proc (handle->proc_cls, resp, MHD_HTTP_OK); GNUNET_RECLAIM_attribute_list_destroy (cl); GNUNET_RECLAIM_presentation_list_destroy (pl); @@ -2665,8 +2685,14 @@ oidc_config_cors (struct GNUNET_REST_RequestHandle *con_handle, // For now, independent of path return all options resp = GNUNET_REST_create_response (NULL); - MHD_add_response_header (resp, "Access-Control-Allow-Methods", allow_methods); - MHD_add_response_header (resp, "Access-Control-Allow-Origin", "*"); + GNUNET_assert (MHD_NO != + MHD_add_response_header (resp, + "Access-Control-Allow-Methods", + allow_methods)); + GNUNET_assert (MHD_NO != + MHD_add_response_header (resp, + "Access-Control-Allow-Origin", + "*")); handle->proc (handle->proc_cls, resp, MHD_HTTP_OK); cleanup_handle (handle); return; diff --git a/src/reclaim/plugin_rest_reclaim.c b/src/reclaim/plugin_rest_reclaim.c index 84456b386..39d24ea61 100644 --- a/src/reclaim/plugin_rest_reclaim.c +++ b/src/reclaim/plugin_rest_reclaim.c @@ -353,14 +353,18 @@ finished_cont (void *cls, int32_t success, const char *emsg) struct MHD_Response *resp; handle->idp_op = NULL; - resp = GNUNET_REST_create_response (emsg); - MHD_add_response_header (resp, "Content-Type", "application/json"); - MHD_add_response_header (resp, "Access-Control-Allow-Methods", allow_methods); if (GNUNET_OK != success) { GNUNET_SCHEDULER_add_now (&do_error, handle); return; } + resp = GNUNET_REST_create_response (emsg); + GNUNET_assert (MHD_NO != MHD_add_response_header (resp, + "Content-Type", + "application/json")); + GNUNET_assert (MHD_NO != MHD_add_response_header (resp, + "Access-Control-Allow-Methods", + allow_methods)); handle->proc (handle->proc_cls, resp, MHD_HTTP_OK); GNUNET_SCHEDULER_add_now (&cleanup_handle, handle); } @@ -372,13 +376,15 @@ delete_finished_cb (void *cls, int32_t success, const char *emsg) struct RequestHandle *handle = cls; struct MHD_Response *resp; - resp = GNUNET_REST_create_response (emsg); - MHD_add_response_header (resp, "Access-Control-Allow-Methods", allow_methods); if (GNUNET_OK != success) { GNUNET_SCHEDULER_add_now (&do_error, handle); return; } + resp = GNUNET_REST_create_response (emsg); + GNUNET_assert (MHD_NO != MHD_add_response_header (resp, + "Access-Control-Allow-Methods", + allow_methods)); handle->proc (handle->proc_cls, resp, MHD_HTTP_OK); GNUNET_SCHEDULER_add_now (&cleanup_handle, handle); } @@ -399,7 +405,10 @@ return_response (void *cls) result_str = json_dumps (handle->resp_object, 0); GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Result %s\n", result_str); resp = GNUNET_REST_create_response (result_str); - MHD_add_response_header (resp, "Access-Control-Allow-Methods", allow_methods); + GNUNET_assert (MHD_NO != + MHD_add_response_header (resp, + "Access-Control-Allow-Methods", + allow_methods)); handle->proc (handle->proc_cls, resp, MHD_HTTP_OK); GNUNET_free (result_str); cleanup_handle (handle); @@ -461,8 +470,8 @@ ticket_collect (void *cls, const struct GNUNET_RECLAIM_Ticket *ticket) static void add_credential_cont (struct GNUNET_REST_RequestHandle *con_handle, - const char *url, - void *cls) + const char *url, + void *cls) { struct RequestHandle *handle = cls; const struct GNUNET_IDENTITY_PrivateKey *identity_priv; @@ -513,7 +522,15 @@ add_credential_cont (struct GNUNET_REST_RequestHandle *con_handle, handle->rest_handle->data, handle->rest_handle->data_size); data_json = json_loads (term_data, JSON_DECODE_ANY, &err); - GNUNET_JSON_parse (data_json, attrspec, NULL, NULL); + if (GNUNET_OK != GNUNET_JSON_parse (data_json, attrspec, NULL, NULL)) + { + json_decref (data_json); + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "Unable to parse JSON from %s\n", + term_data); + GNUNET_SCHEDULER_add_now (&do_error, handle); + return; + } json_decref (data_json); if (NULL == attribute) { @@ -530,11 +547,11 @@ add_credential_cont (struct GNUNET_REST_RequestHandle *con_handle, GNUNET_RECLAIM_id_generate (&attribute->id); exp = GNUNET_TIME_UNIT_HOURS; handle->idp_op = GNUNET_RECLAIM_credential_store (idp, - identity_priv, - attribute, - &exp, - &finished_cont, - handle); + identity_priv, + attribute, + &exp, + &finished_cont, + handle); GNUNET_JSON_parse_free (attrspec); } @@ -545,8 +562,8 @@ add_credential_cont (struct GNUNET_REST_RequestHandle *con_handle, */ static void cred_collect (void *cls, - const struct GNUNET_IDENTITY_PublicKey *identity, - const struct GNUNET_RECLAIM_Credential *cred) + const struct GNUNET_IDENTITY_PublicKey *identity, + const struct GNUNET_RECLAIM_Credential *cred) { struct RequestHandle *handle = cls; struct GNUNET_RECLAIM_AttributeList *attrs; @@ -565,8 +582,8 @@ cred_collect (void *cls, attrs = GNUNET_RECLAIM_credential_get_attributes (cred); issuer = GNUNET_RECLAIM_credential_get_issuer (cred); tmp_value = GNUNET_RECLAIM_credential_value_to_string (cred->type, - cred->data, - cred->data_size); + cred->data, + cred->data_size); cred_obj = json_object (); json_object_set_new (cred_obj, "value", json_string (tmp_value)); json_object_set_new (cred_obj, "name", json_string (cred->name)); @@ -578,7 +595,7 @@ cred_collect (void *cls, GNUNET_free (issuer); } if (GNUNET_OK == GNUNET_RECLAIM_credential_get_expiration (cred, - &exp)) + &exp)) { json_object_set_new (cred_obj, "expiration", json_integer ( exp.abs_value_us)); @@ -613,7 +630,8 @@ cred_collect (void *cls, json_object_set_new (cred_obj, "attributes", attr_arr); } json_array_append_new (handle->resp_object, cred_obj); - GNUNET_RECLAIM_attribute_list_destroy (attrs); + if (NULL != attrs) + GNUNET_RECLAIM_attribute_list_destroy (attrs); GNUNET_RECLAIM_get_credentials_next (handle->cred_it); } @@ -627,8 +645,8 @@ cred_collect (void *cls, */ static void list_credential_cont (struct GNUNET_REST_RequestHandle *con_handle, - const char *url, - void *cls) + const char *url, + void *cls) { struct RequestHandle *handle = cls; const struct GNUNET_IDENTITY_PrivateKey *priv_key; @@ -664,14 +682,14 @@ list_credential_cont (struct GNUNET_REST_RequestHandle *con_handle, } priv_key = GNUNET_IDENTITY_ego_get_private_key (ego_entry->ego); handle->cred_it = GNUNET_RECLAIM_get_credentials_start (idp, - priv_key, - &collect_error_cb, - handle, - &cred_collect, - handle, - & - collect_finished_cb, - handle); + priv_key, + &collect_error_cb, + handle, + &cred_collect, + handle, + & + collect_finished_cb, + handle); } @@ -684,8 +702,8 @@ list_credential_cont (struct GNUNET_REST_RequestHandle *con_handle, */ static void delete_credential_cont (struct GNUNET_REST_RequestHandle *con_handle, - const char *url, - void *cls) + const char *url, + void *cls) { struct RequestHandle *handle = cls; const struct GNUNET_IDENTITY_PrivateKey *priv_key; @@ -734,10 +752,10 @@ delete_credential_cont (struct GNUNET_REST_RequestHandle *con_handle, GNUNET_STRINGS_string_to_data (id, strlen (id), &attr.id, sizeof(attr.id)); attr.name = ""; handle->idp_op = GNUNET_RECLAIM_credential_delete (idp, - priv_key, - &attr, - &delete_finished_cb, - handle); + priv_key, + &attr, + &delete_finished_cb, + handle); GNUNET_free (identity_id_str); } @@ -900,8 +918,8 @@ parse_jwt (const struct GNUNET_RECLAIM_Credential *cred, json_error_t *json_err = NULL; jwt_string = GNUNET_RECLAIM_credential_value_to_string (cred->type, - cred->data, - cred->data_size); + cred->data, + cred->data_size); char *jwt_body = strtok (jwt_string, delim); jwt_body = strtok (NULL, delim); GNUNET_STRINGS_base64_decode (jwt_body, strlen (jwt_body), @@ -1424,25 +1442,24 @@ rest_identity_process_request (struct GNUNET_REST_RequestHandle *rest_handle, static const struct GNUNET_REST_RequestHandler handlers[] = { { MHD_HTTP_METHOD_GET, GNUNET_REST_API_NS_RECLAIM_ATTRIBUTES, &list_attribute_cont }, - { MHD_HTTP_METHOD_POST, - GNUNET_REST_API_NS_RECLAIM_ATTRIBUTES, &add_attribute_cont }, - { MHD_HTTP_METHOD_DELETE, - GNUNET_REST_API_NS_RECLAIM_ATTRIBUTES, &delete_attribute_cont }, - { MHD_HTTP_METHOD_GET, - GNUNET_REST_API_NS_RECLAIM_CREDENTIAL, &list_credential_cont }, - { MHD_HTTP_METHOD_POST, - GNUNET_REST_API_NS_RECLAIM_CREDENTIAL, &add_credential_cont }, - { MHD_HTTP_METHOD_DELETE, - GNUNET_REST_API_NS_RECLAIM_CREDENTIAL, &delete_credential_cont }, - { MHD_HTTP_METHOD_GET, - GNUNET_REST_API_NS_IDENTITY_TICKETS, &list_tickets_cont }, - { MHD_HTTP_METHOD_POST, - GNUNET_REST_API_NS_IDENTITY_REVOKE, &revoke_ticket_cont }, - { MHD_HTTP_METHOD_POST, - GNUNET_REST_API_NS_IDENTITY_CONSUME, &consume_ticket_cont }, - { MHD_HTTP_METHOD_OPTIONS, GNUNET_REST_API_NS_RECLAIM, &options_cont }, - GNUNET_REST_HANDLER_END - }; + { MHD_HTTP_METHOD_POST, + GNUNET_REST_API_NS_RECLAIM_ATTRIBUTES, &add_attribute_cont }, + { MHD_HTTP_METHOD_DELETE, + GNUNET_REST_API_NS_RECLAIM_ATTRIBUTES, &delete_attribute_cont }, + { MHD_HTTP_METHOD_GET, + GNUNET_REST_API_NS_RECLAIM_CREDENTIAL, &list_credential_cont }, + { MHD_HTTP_METHOD_POST, + GNUNET_REST_API_NS_RECLAIM_CREDENTIAL, &add_credential_cont }, + { MHD_HTTP_METHOD_DELETE, + GNUNET_REST_API_NS_RECLAIM_CREDENTIAL, &delete_credential_cont }, + { MHD_HTTP_METHOD_GET, + GNUNET_REST_API_NS_IDENTITY_TICKETS, &list_tickets_cont }, + { MHD_HTTP_METHOD_POST, + GNUNET_REST_API_NS_IDENTITY_REVOKE, &revoke_ticket_cont }, + { MHD_HTTP_METHOD_POST, + GNUNET_REST_API_NS_IDENTITY_CONSUME, &consume_ticket_cont }, + { MHD_HTTP_METHOD_OPTIONS, GNUNET_REST_API_NS_RECLAIM, &options_cont }, + GNUNET_REST_HANDLER_END}; handle->response_code = 0; handle->timeout = GNUNET_TIME_UNIT_FOREVER_REL; diff --git a/src/reclaim/reclaim_api.c b/src/reclaim/reclaim_api.c index f4f2b946a..c08cc868c 100644 --- a/src/reclaim/reclaim_api.c +++ b/src/reclaim/reclaim_api.c @@ -651,18 +651,15 @@ handle_consume_ticket_result (void *cls, le->attribute, NULL); } } - if (NULL != attrs) - GNUNET_RECLAIM_attribute_list_destroy (attrs); - if (NULL != pl) - GNUNET_RECLAIM_presentation_list_destroy (pl); - attrs = NULL; - pl = NULL; } op->atr_cb (op->cls, NULL, NULL, NULL); } + if (NULL != attrs) + GNUNET_RECLAIM_attribute_list_destroy (attrs); + if (NULL != pl) + GNUNET_RECLAIM_presentation_list_destroy (pl); GNUNET_CONTAINER_DLL_remove (h->op_head, h->op_tail, op); free_op (op); - GNUNET_free (attrs); return; } GNUNET_assert (0); @@ -804,7 +801,7 @@ check_credential_result (void *cls, const struct CredentialResultMessage *msg) */ static void handle_credential_result (void *cls, const struct - CredentialResultMessage *msg) + CredentialResultMessage *msg) { static struct GNUNET_IDENTITY_PrivateKey identity_dummy; struct GNUNET_RECLAIM_Handle *h = cls; @@ -871,6 +868,7 @@ handle_credential_result (void *cls, const struct GNUNET_assert (0); } + /** * Handle an incoming message of type * #GNUNET_MESSAGE_TYPE_RECLAIM_TICKET_RESULT @@ -925,7 +923,7 @@ handle_ticket_result (void *cls, const struct TicketResultMessage *msg) if (NULL != op) { if (0 < pres_len) - pres = GNUNET_RECLAIM_presentation_list_deserialize ((char*)&msg[1], + pres = GNUNET_RECLAIM_presentation_list_deserialize ((char*) &msg[1], pres_len); GNUNET_CONTAINER_DLL_remove (handle->op_head, handle->op_tail, op); if (0 == @@ -1485,7 +1483,7 @@ GNUNET_RECLAIM_get_credentials_start ( */ void GNUNET_RECLAIM_get_credentials_next (struct - GNUNET_RECLAIM_CredentialIterator *ait) + GNUNET_RECLAIM_CredentialIterator *ait) { struct GNUNET_RECLAIM_Handle *h = ait->h; struct CredentialIterationNextMessage *msg; @@ -1507,7 +1505,7 @@ GNUNET_RECLAIM_get_credentials_next (struct */ void GNUNET_RECLAIM_get_credentials_stop (struct - GNUNET_RECLAIM_CredentialIterator *ait) + GNUNET_RECLAIM_CredentialIterator *ait) { struct GNUNET_RECLAIM_Handle *h = ait->h; struct GNUNET_MQ_Envelope *env; -- cgit v1.2.3