From 2aa1df730ce3f12454909bbc4a19b6a5d480c7ff Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Tue, 5 Mar 2024 11:57:26 -0800 Subject: [PATCH 1/6] Fix GeoJSON string crash --- src/main/conversions.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/conversions.c b/src/main/conversions.c index fadcf3a21..6a75d7564 100644 --- a/src/main/conversions.c +++ b/src/main/conversions.c @@ -1461,6 +1461,12 @@ as_status do_val_to_pyobject(AerospikeClient *self, as_error *err, as_geojson *gp = as_geojson_fromval(val); char *locstr = as_geojson_get(gp); PyObject *py_locstr = PyUnicode_FromString(locstr); + if (!py_locstr) { + // An invalid C string was passed (cannot be parsed into unicode) + as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Unable to parse GeoJSON string"); + return err->code; + } PyObject *py_loads = AerospikeGeospatial_DoLoads(py_locstr, err); Py_DECREF(py_locstr); if (err->code != AEROSPIKE_OK) { From 6fe9b0787032fa063ca346232eb2182e2de017e8 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Tue, 12 Mar 2024 09:36:51 -0700 Subject: [PATCH 2/6] Prevent more crashes (not all of them yet) --- src/main/conversions.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/main/conversions.c b/src/main/conversions.c index 6a75d7564..5c9218dd9 100644 --- a/src/main/conversions.c +++ b/src/main/conversions.c @@ -77,6 +77,11 @@ as_status as_udf_file_to_pyobject(as_error *err, as_udf_file *entry, *py_file = PyDict_New(); PyObject *py_name = PyUnicode_FromString(entry->name); + if (!py_name) { + Py_XDECREF(py_file); + return as_error_update(err, AEROSPIKE_ERR, + "Unable to parse UDF file name"); + } PyDict_SetItemString(*py_file, "name", py_name); Py_DECREF(py_name); @@ -178,6 +183,11 @@ as_status as_user_array_to_pyobject(as_error *err, as_user **users, for (i = 0; i < users_size; i++) { PyObject *py_user = PyUnicode_FromString(users[i]->name); + if (!py_user) { + Py_XDECREF(py_users); + return as_error_update(err, AEROSPIKE_ERR, + "Unable to parse user name at index %d", i); + } PyObject *py_roles = PyList_New(0); str_array_of_roles_to_py_list(err, users[i]->roles_size, users[i]->roles, py_roles); @@ -296,6 +306,11 @@ as_status as_role_array_to_pyobject_old(as_error *err, as_role **roles, for (i = 0; i < roles_size; i++) { PyObject *py_role = PyUnicode_FromString(roles[i]->name); + if (!py_role) { + Py_XDECREF(py_roles); + return as_error_update(&err, AEROSPIKE_ERR_CLUSTER, + "Unable to parse role name at index %d", i); + } PyObject *py_privileges = PyList_New(0); as_privilege_to_pyobject(err, roles[i]->privileges, py_privileges, @@ -630,7 +645,18 @@ as_status as_privilege_to_pyobject(as_error *err, as_privilege privileges[], PyObject *py_code = NULL; for (int i = 0; i < privilege_size; i++) { py_ns = PyUnicode_FromString(privileges[i].ns); + if (!py_ns) { + return as_error_update( + err, AEROSPIKE_ERR, + "Unable to parse namespace for privilege at index %d", i); + } py_set = PyUnicode_FromString(privileges[i].set); + if (!py_set) { + Py_DECREF(py_ns); + return as_error_update( + err, AEROSPIKE_ERR, + "Unable to parse set for privilege at index %d", i); + } py_code = PyLong_FromLong(privileges[i].code); PyObject *py_privilege = PyDict_New(); From 93ca495d41bf1481e210b55793a1ea8c7e28bd38 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 21 Mar 2024 16:54:16 -0700 Subject: [PATCH 3/6] Fix more potential crashes --- src/main/client/get_cdtctx_base64.c | 4 ++++ src/main/client/get_expression_base64.c | 4 ++++ src/main/client/info_random_node.c | 5 +++++ src/main/client/info_single_node.c | 5 +++++ src/main/client/set_xdr_filter.c | 5 +++++ src/main/conversions.c | 11 ++++++++++- src/main/geospatial/type.c | 4 ++++ src/main/serializer.c | 6 ++++++ 8 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/main/client/get_cdtctx_base64.c b/src/main/client/get_cdtctx_base64.c index 5b3e2cc25..5fdd96a77 100644 --- a/src/main/client/get_cdtctx_base64.c +++ b/src/main/client/get_cdtctx_base64.c @@ -112,6 +112,10 @@ PyObject *AerospikeClient_GetCDTCTXBase64(AerospikeClient *self, PyObject *args, } py_response = PyUnicode_FromString((const char *)base64); + if (!py_response) { + as_error_update(&err, AEROSPIKE_ERR_CLIENT, + "Unable to parse base64 string"); + } CLEANUP: diff --git a/src/main/client/get_expression_base64.c b/src/main/client/get_expression_base64.c index 84ecfe898..43dc0128a 100644 --- a/src/main/client/get_expression_base64.c +++ b/src/main/client/get_expression_base64.c @@ -76,6 +76,10 @@ PyObject *AerospikeClient_GetExpressionBase64(AerospikeClient *self, base64_filter = as_exp_compile_b64(exp_list_p); py_response = PyUnicode_FromString((const char *)base64_filter); + if (!py_response) { + as_error_update(&err, AEROSPIKE_ERR_CLIENT, + "Unable to parse base64 string"); + } CLEANUP: diff --git a/src/main/client/info_random_node.c b/src/main/client/info_random_node.c index 6cfb3a16f..722facb93 100644 --- a/src/main/client/info_random_node.c +++ b/src/main/client/info_random_node.c @@ -86,6 +86,11 @@ static PyObject *AerospikeClient_InfoRandomNode_Invoke(as_error *err, if (err->code == AEROSPIKE_OK) { if (response_p != NULL && status == AEROSPIKE_OK) { py_response = PyUnicode_FromString(response_p); + if (!py_response) { + as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Unable to parse info operation response"); + goto CLEANUP; + } } else if (response_p == NULL) { as_error_update(err, AEROSPIKE_ERR_CLIENT, diff --git a/src/main/client/info_single_node.c b/src/main/client/info_single_node.c index 0634dcd37..fb759a5b6 100644 --- a/src/main/client/info_single_node.c +++ b/src/main/client/info_single_node.c @@ -106,6 +106,11 @@ static PyObject *AerospikeClient_InfoSingleNode_Invoke(as_error *err, if (err->code == AEROSPIKE_OK) { if (response_p != NULL && status == AEROSPIKE_OK) { py_response = PyUnicode_FromString(response_p); + if (!py_response) { + as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Unable to parse info operation response"); + goto CLEANUP; + } } else if (response_p == NULL) { as_error_update(err, AEROSPIKE_ERR_CLIENT, diff --git a/src/main/client/set_xdr_filter.c b/src/main/client/set_xdr_filter.c index 9c5723889..bc3b78819 100644 --- a/src/main/client/set_xdr_filter.c +++ b/src/main/client/set_xdr_filter.c @@ -138,6 +138,11 @@ PyObject *AerospikeClient_SetXDRFilter(AerospikeClient *self, PyObject *args, if (err.code == AEROSPIKE_OK) { if (response_p != NULL && status == AEROSPIKE_OK) { py_response = PyUnicode_FromString(response_p); + if (!py_response) { + as_error_update(&err, AEROSPIKE_ERR_CLIENT, + "Unable to parse info operation response"); + goto CLEANUP; + } } else if (response_p == NULL) { as_error_update(&err, AEROSPIKE_ERR_CLIENT, diff --git a/src/main/conversions.c b/src/main/conversions.c index 5c9218dd9..40763ec4e 100644 --- a/src/main/conversions.c +++ b/src/main/conversions.c @@ -308,7 +308,7 @@ as_status as_role_array_to_pyobject_old(as_error *err, as_role **roles, PyObject *py_role = PyUnicode_FromString(roles[i]->name); if (!py_role) { Py_XDECREF(py_roles); - return as_error_update(&err, AEROSPIKE_ERR_CLUSTER, + return as_error_update(err, AEROSPIKE_ERR_CLUSTER, "Unable to parse role name at index %d", i); } PyObject *py_privileges = PyList_New(0); @@ -1855,10 +1855,19 @@ as_status key_to_pyobject(as_error *err, const as_key *key, PyObject **obj) if (strlen(key->ns) > 0) { py_namespace = PyUnicode_FromString(key->ns); + if (!py_namespace) { + return as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Unable to parse key namespace"); + } } if (strlen(key->set) > 0) { py_set = PyUnicode_FromString(key->set); + if (!py_set) { + Py_DECREF(py_namespace); + return as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Unable to parse key namespace"); + } } if (key->valuep) { diff --git a/src/main/geospatial/type.c b/src/main/geospatial/type.c index 880ee9a56..c09bf2662 100644 --- a/src/main/geospatial/type.c +++ b/src/main/geospatial/type.c @@ -172,6 +172,10 @@ AerospikeGeospatial *self; } py_return = PyUnicode_FromString(new_repr_str); + if (!py_return) { + as_error_update(&err, AEROSPIKE_ERR_CLIENT, + "Unable to parse GeoJSON string into a unicode object"); + } Py_XDECREF(initresult); free(new_repr_str); return py_return; diff --git a/src/main/serializer.c b/src/main/serializer.c index bee1e85f4..c03b6b1c5 100644 --- a/src/main/serializer.c +++ b/src/main/serializer.c @@ -231,6 +231,12 @@ void execute_user_callback(user_serializer_callback *user_callback_info, char *bytes_val_p = (char *)bytes_pointer->value; py_value = PyUnicode_FromStringAndSize(bytes_val_p, as_bytes_size(*bytes)); + if (!py_value) { + as_error_update(error_p, AEROSPIKE_ERR_CLIENT, + "Failed to convert bytes to Python unicode object"); + Py_DECREF(py_arglist); + goto CLEANUP; + } if (PyTuple_SetItem(py_arglist, 0, py_value) != 0) { Py_DECREF(py_arglist); goto CLEANUP; From 8acd3183ec2a7d3820fdc3eb8e1d19e5db7704d2 Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Wed, 25 Sep 2024 16:35:20 -0700 Subject: [PATCH 4/6] Clear error indicator if error returned --- src/main/client/get_cdtctx_base64.c | 1 + src/main/client/get_expression_base64.c | 1 + src/main/client/info_random_node.c | 1 + src/main/client/info_single_node.c | 1 + src/main/client/set_xdr_filter.c | 1 + src/main/conversions.c | 29 ++++++++++++++++++++++--- src/main/geospatial/type.c | 14 +++++++----- src/main/serializer.c | 1 + 8 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/main/client/get_cdtctx_base64.c b/src/main/client/get_cdtctx_base64.c index 5fdd96a77..c8bbf3f61 100644 --- a/src/main/client/get_cdtctx_base64.c +++ b/src/main/client/get_cdtctx_base64.c @@ -113,6 +113,7 @@ PyObject *AerospikeClient_GetCDTCTXBase64(AerospikeClient *self, PyObject *args, py_response = PyUnicode_FromString((const char *)base64); if (!py_response) { + PyErr_Clear(); as_error_update(&err, AEROSPIKE_ERR_CLIENT, "Unable to parse base64 string"); } diff --git a/src/main/client/get_expression_base64.c b/src/main/client/get_expression_base64.c index 43dc0128a..83acc91f1 100644 --- a/src/main/client/get_expression_base64.c +++ b/src/main/client/get_expression_base64.c @@ -77,6 +77,7 @@ PyObject *AerospikeClient_GetExpressionBase64(AerospikeClient *self, py_response = PyUnicode_FromString((const char *)base64_filter); if (!py_response) { + PyErr_Clear(); as_error_update(&err, AEROSPIKE_ERR_CLIENT, "Unable to parse base64 string"); } diff --git a/src/main/client/info_random_node.c b/src/main/client/info_random_node.c index 722facb93..038ec312d 100644 --- a/src/main/client/info_random_node.c +++ b/src/main/client/info_random_node.c @@ -87,6 +87,7 @@ static PyObject *AerospikeClient_InfoRandomNode_Invoke(as_error *err, if (response_p != NULL && status == AEROSPIKE_OK) { py_response = PyUnicode_FromString(response_p); if (!py_response) { + PyErr_Clear(); as_error_update(err, AEROSPIKE_ERR_CLIENT, "Unable to parse info operation response"); goto CLEANUP; diff --git a/src/main/client/info_single_node.c b/src/main/client/info_single_node.c index fb759a5b6..32c432b4b 100644 --- a/src/main/client/info_single_node.c +++ b/src/main/client/info_single_node.c @@ -107,6 +107,7 @@ static PyObject *AerospikeClient_InfoSingleNode_Invoke(as_error *err, if (response_p != NULL && status == AEROSPIKE_OK) { py_response = PyUnicode_FromString(response_p); if (!py_response) { + PyErr_Clear(); as_error_update(err, AEROSPIKE_ERR_CLIENT, "Unable to parse info operation response"); goto CLEANUP; diff --git a/src/main/client/set_xdr_filter.c b/src/main/client/set_xdr_filter.c index bc3b78819..846092550 100644 --- a/src/main/client/set_xdr_filter.c +++ b/src/main/client/set_xdr_filter.c @@ -139,6 +139,7 @@ PyObject *AerospikeClient_SetXDRFilter(AerospikeClient *self, PyObject *args, if (response_p != NULL && status == AEROSPIKE_OK) { py_response = PyUnicode_FromString(response_p); if (!py_response) { + PyErr_Clear(); as_error_update(&err, AEROSPIKE_ERR_CLIENT, "Unable to parse info operation response"); goto CLEANUP; diff --git a/src/main/conversions.c b/src/main/conversions.c index 3a5b02d64..3867049fa 100644 --- a/src/main/conversions.c +++ b/src/main/conversions.c @@ -77,10 +77,16 @@ as_status as_udf_file_to_pyobject(as_error *err, as_udf_file *entry, as_error_reset(err); *py_file = PyDict_New(); + if (*py_file == NULL) { + PyErr_Clear(); + return as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Unable to create UDF file object"); + } PyObject *py_name = PyUnicode_FromString(entry->name); if (!py_name) { - Py_XDECREF(py_file); + PyErr_Clear(); + Py_DECREF(py_file); return as_error_update(err, AEROSPIKE_ERR, "Unable to parse UDF file name"); } @@ -182,11 +188,18 @@ as_status as_user_array_to_pyobject(as_error *err, as_user **users, int i; PyObject *py_users = PyDict_New(); + if (py_users == NULL) { + PyErr_Clear(); + return as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Unable to create users object"); + } + for (i = 0; i < users_size; i++) { PyObject *py_user = PyUnicode_FromString(users[i]->name); if (!py_user) { - Py_XDECREF(py_users); + PyErr_Clear(); + Py_DECREF(py_users); return as_error_update(err, AEROSPIKE_ERR, "Unable to parse user name at index %d", i); } @@ -305,11 +318,18 @@ as_status as_role_array_to_pyobject_old(as_error *err, as_role **roles, int i; PyObject *py_roles = PyDict_New(); + if (py_roles == NULL) { + PyErr_Clear(); + return as_error_update(err, AEROSPIKE_ERR_CLIENT, + "Unable to create roles object"); + } + for (i = 0; i < roles_size; i++) { PyObject *py_role = PyUnicode_FromString(roles[i]->name); if (!py_role) { - Py_XDECREF(py_roles); + PyErr_Clear(); + Py_DECREF(py_roles); return as_error_update(err, AEROSPIKE_ERR_CLUSTER, "Unable to parse role name at index %d", i); } @@ -1809,6 +1829,7 @@ as_status do_val_to_pyobject(AerospikeClient *self, as_error *err, PyObject *py_locstr = PyUnicode_FromString(locstr); if (!py_locstr) { // An invalid C string was passed (cannot be parsed into unicode) + PyErr_Clear(); as_error_update(err, AEROSPIKE_ERR_CLIENT, "Unable to parse GeoJSON string"); return err->code; @@ -2176,6 +2197,7 @@ as_status key_to_pyobject(as_error *err, const as_key *key, PyObject **obj) if (strlen(key->ns) > 0) { py_namespace = PyUnicode_FromString(key->ns); if (!py_namespace) { + PyErr_Clear(); return as_error_update(err, AEROSPIKE_ERR_CLIENT, "Unable to parse key namespace"); } @@ -2184,6 +2206,7 @@ as_status key_to_pyobject(as_error *err, const as_key *key, PyObject **obj) if (strlen(key->set) > 0) { py_set = PyUnicode_FromString(key->set); if (!py_set) { + PyErr_Clear(); Py_DECREF(py_namespace); return as_error_update(err, AEROSPIKE_ERR_CLIENT, "Unable to parse key namespace"); diff --git a/src/main/geospatial/type.c b/src/main/geospatial/type.c index 144168dca..17b973b4f 100644 --- a/src/main/geospatial/type.c +++ b/src/main/geospatial/type.c @@ -160,8 +160,15 @@ AerospikeGeospatial *self; snprintf(new_repr_str, strlen(initresult_str) + 3, "\'%s\'", initresult_str); -CLEANUP: + py_return = PyUnicode_FromString(new_repr_str); + if (!py_return) { + PyErr_Clear(); + as_error_update(&err, AEROSPIKE_ERR_CLIENT, + "Unable to parse GeoJSON string into a unicode object"); + } +CLEANUP: + // TODO: need to free initresult? // If an error occurred, tell Python. if (err.code != AEROSPIKE_OK) { raise_exception(&err); @@ -171,11 +178,6 @@ AerospikeGeospatial *self; return NULL; } - py_return = PyUnicode_FromString(new_repr_str); - if (!py_return) { - as_error_update(&err, AEROSPIKE_ERR_CLIENT, - "Unable to parse GeoJSON string into a unicode object"); - } Py_XDECREF(initresult); free(new_repr_str); return py_return; diff --git a/src/main/serializer.c b/src/main/serializer.c index 6fe07d79b..0ae25497e 100644 --- a/src/main/serializer.c +++ b/src/main/serializer.c @@ -232,6 +232,7 @@ void execute_user_callback(user_serializer_callback *user_callback_info, py_value = PyUnicode_FromStringAndSize(bytes_val_p, as_bytes_size(*bytes)); if (!py_value) { + PyErr_Clear(); as_error_update(error_p, AEROSPIKE_ERR_CLIENT, "Failed to convert bytes to Python unicode object"); Py_DECREF(py_arglist); From 0489e73b9a8e1321e5f0c736b2bf48626efc336d Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 26 Sep 2024 08:55:34 -0700 Subject: [PATCH 5/6] fix ref count leak --- src/main/geospatial/type.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/geospatial/type.c b/src/main/geospatial/type.c index 17b973b4f..013c23f1a 100644 --- a/src/main/geospatial/type.c +++ b/src/main/geospatial/type.c @@ -168,7 +168,7 @@ AerospikeGeospatial *self; } CLEANUP: - // TODO: need to free initresult? + Py_XDECREF(initresult); // If an error occurred, tell Python. if (err.code != AEROSPIKE_OK) { raise_exception(&err); @@ -178,7 +178,6 @@ AerospikeGeospatial *self; return NULL; } - Py_XDECREF(initresult); free(new_repr_str); return py_return; } From 651635f5ae00d2c6f60f3b436cd53972a860981c Mon Sep 17 00:00:00 2001 From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com> Date: Thu, 10 Oct 2024 12:45:05 -0700 Subject: [PATCH 6/6] refactor --- src/main/geospatial/type.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/geospatial/type.c b/src/main/geospatial/type.c index 013c23f1a..b821116a6 100644 --- a/src/main/geospatial/type.c +++ b/src/main/geospatial/type.c @@ -169,16 +169,15 @@ AerospikeGeospatial *self; CLEANUP: Py_XDECREF(initresult); + if (new_repr_str) { + free(new_repr_str); + } // If an error occurred, tell Python. if (err.code != AEROSPIKE_OK) { raise_exception(&err); - if (new_repr_str) { - free(new_repr_str); - } return NULL; } - free(new_repr_str); return py_return; }