Skip to content

fix(asap): avoid double-free on malformed JSON-RPC results #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
cursor wants to merge 1 commit into
base: main
Choose a base branch
from
+59 −2
Draft

fix(asap): avoid double-free on malformed JSON-RPC results #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/asap/envelope.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ int asap_envelope_parse_jsonrpc_response(const char *json, asap_envelope_t *out,
if (errmsg && errlen && cJSON_IsString(m) && m->valuestring) (void)snprintf(errmsg, errlen, "%s", m->valuestring);
cJSON_Delete(tmp_err);
} else if (errmsg && errlen) (void)snprintf(errmsg, errlen, "Invalid result envelope");
cJSON_Delete(rpc_id);
/* parse_fail already freed rpc_id (unlike the success path below). */
cJSON_Delete(root);
return -1;
}
Expand Down
5 changes: 4 additions & 1 deletion src/asap/envelope.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ int asap_envelope_parse(const char *json, asap_envelope_t *out, cJSON **err_out)
* jsonrpc_request_id.
*
* @param obj Object with ASAP envelope members (not NULL)
* @param rpc_id Request id for error echo (not consumed)
* @param rpc_id Request id for error echo. On validation failure the
* implementation frees this pointer (via parse_fail). On
* success it is not consumed; the caller may assign it to
* out->jsonrpc_request_id.
* @param out Envelope; must be zeroed (#asap_envelope_init) or cleared first
* because the implementation clears the struct on entry.
* @param err_out Optional error root
Expand Down
54 changes: 54 additions & 0 deletions tests/test_asap_client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,27 @@ static int test_parse_response_jsonrpc_error(void)
return 0;
}

/* HTTP 200 result missing required envelope fields used to double-free rpc_id. */
static int test_parse_response_missing_payload(void)
{
const char *json = "{\"jsonrpc\":\"2.0\","
"\"id\":\"1\","
"\"result\":{"
"\"id\":\"t1\","
"\"asap_version\":\"2.1\","
"\"sender\":\"a\",\"recipient\":\"b\","
"\"payload_type\":\"task.response\""
"}}";
char err[128];
asap_envelope_t out;
asap_envelope_init(&out);
err[0] = 0;
ASSERT(asap_envelope_parse_jsonrpc_response(json, &out, err, sizeof err) == -1);
ASSERT(strstr(err, "payload") != NULL);
asap_envelope_clear(&out);
return 0;
}

static int test_config_defaults(void)
{
asap_client_config_t c;
Expand Down Expand Up @@ -279,6 +300,16 @@ static const char live_ok_body[] =
"},"
"\"id\":null}";

static const char live_missing_payload_body[] =
"{\"jsonrpc\":\"2.0\","
"\"id\":\"1\","
"\"result\":{"
"\"id\":\"t1\","
"\"asap_version\":\"2.1\","
"\"sender\":\"a\",\"recipient\":\"b\","
"\"payload_type\":\"task.response\""
"}}";

static const char live_ok_body_matching_req[] =
"{\"jsonrpc\":\"2.0\","
"\"result\":{"
Expand Down Expand Up @@ -386,6 +417,27 @@ static int test_live_http_non_two_hundred(void)
return 0;
}

static int test_live_http_malformed_result_envelope(void)
{
asap_envelope_t env;
asap_envelope_t resp;
struct tiny_http_srv srv;
char err[256];
char url[96];
ASSERT(fill_min_task_request(&env) == 0);
ASSERT(tiny_http_start(&srv, 200L, live_missing_payload_body, strlen(live_missing_payload_body)) == 0);
asap_envelope_init(&resp);
err[0] = '\0';
ASSERT(snprintf(url, sizeof url, "http://127.0.0.1:%hu/", srv.bind_port) < (int)sizeof url);
ASSERT(asap_client_send_task(url, NULL, ASAP_DEFAULT_JSONRPC_METHOD, &env, NULL, NULL, &resp,
err, sizeof err) == -1);
ASSERT(strstr(err, "payload") != NULL);
asap_envelope_clear(&env);
asap_envelope_clear(&resp);
tiny_http_join(&srv);
return 0;
}

static int test_empty_jsonrpc_method_uses_default(void)
{
asap_envelope_t env;
Expand Down Expand Up @@ -442,11 +494,13 @@ int main(int argc, char **argv)
if (test_request_roundtrip_string() != 0) { fprintf(stderr, "test_request_roundtrip_string failed\n"); failed++; }
if (test_parse_response_ok() != 0) { fprintf(stderr, "test_parse_response_ok failed\n"); failed++; }
if (test_parse_response_jsonrpc_error() != 0) { fprintf(stderr, "test_parse_response_jsonrpc_error failed\n"); failed++; }
if (test_parse_response_missing_payload() != 0) { fprintf(stderr, "test_parse_response_missing_payload failed\n"); failed++; }
if (test_config_defaults() != 0) { fprintf(stderr, "test_config_defaults failed\n"); failed++; }
if (test_config_from_config_timeout() != 0) { fprintf(stderr, "test_config_from_config_timeout failed\n"); failed++; }
if (test_send_invalid_arguments() != 0) { fprintf(stderr, "test_send_invalid_arguments failed\n"); failed++; }
if (test_live_http_roundtrip_success() != 0) { fprintf(stderr, "test_live_http_roundtrip_success failed\n"); failed++; }
if (test_live_http_non_two_hundred() != 0) { fprintf(stderr, "test_live_http_non_two_hundred failed\n"); failed++; }
if (test_live_http_malformed_result_envelope() != 0) { fprintf(stderr, "test_live_http_malformed_result_envelope failed\n"); failed++; }
if (test_empty_jsonrpc_method_uses_default() != 0) { fprintf(stderr, "test_empty_jsonrpc_method_uses_default failed\n"); failed++; }
if (test_send_with_explicit_jsonrpc_request_id() != 0) { fprintf(stderr, "test_send_with_explicit_jsonrpc_request_id failed\n"); failed++; }
if (test_send_fails_no_server() != 0) { fprintf(stderr, "test_send_fails_no_server failed\n"); failed++; }
Expand Down
Loading