diff options
| -rw-r--r-- | ChangeLog.d/bugfix/ZBX-17837 | 1 | ||||
| -rw-r--r-- | src/libs/zbxembed/httprequest.c | 74 | ||||
| -rw-r--r-- | src/libs/zbxembed/zabbix.c | 19 | ||||
| -rw-r--r-- | src/zabbix_js/zabbix_js.c | 3 |
4 files changed, 70 insertions, 27 deletions
diff --git a/ChangeLog.d/bugfix/ZBX-17837 b/ChangeLog.d/bugfix/ZBX-17837 new file mode 100644 index 00000000000..02c41f114b4 --- /dev/null +++ b/ChangeLog.d/bugfix/ZBX-17837 @@ -0,0 +1 @@ +.......PS. [ZBX-17837] fixed memory leaks when handling CurlHttpRequest errors (Andris, wiper) diff --git a/src/libs/zbxembed/httprequest.c b/src/libs/zbxembed/httprequest.c index 1ee15ef7a58..972cce623b4 100644 --- a/src/libs/zbxembed/httprequest.c +++ b/src/libs/zbxembed/httprequest.c @@ -38,11 +38,16 @@ typedef struct } zbx_es_httprequest_t; +/* ZBX_CURL_SETOPT() macro is a code snippet to make code shorter and facilitate resource deallocation */ +/* in case of error. Be careful with using ZBX_CURL_SETOPT(), duk_push_error_object() and duk_error() */ +/* in functions - it is easy to get memory leaks because duk_error() causes longjmp(). */ +/* Note that the caller of ZBX_CURL_SETOPT() must define variable 'int err_index' and label 'out'. */ #define ZBX_CURL_SETOPT(ctx, handle, opt, value, err) \ if (CURLE_OK != (err = curl_easy_setopt(handle, opt, value))) \ { \ - return duk_error(ctx, DUK_RET_TYPE_ERROR, "cannot set cURL option " #opt ": %s.", \ - curl_easy_strerror(err)); \ + err_index = duk_push_error_object(ctx, DUK_RET_EVAL_ERROR, \ + "cannot set cURL option " #opt ": %s.", curl_easy_strerror(err)); \ + goto out; \ } static size_t curl_write_cb(void *ptr, size_t size, size_t nmemb, void *userdata) @@ -115,13 +120,16 @@ static duk_ret_t es_httprequest_ctor(duk_context *ctx) zbx_es_httprequest_t *request; CURLcode err; zbx_es_env_t *env; + int err_index = -1; if (!duk_is_constructor_call(ctx)) return DUK_RET_TYPE_ERROR; duk_push_global_stash(ctx); + if (1 != duk_get_prop_string(ctx, -1, "\xff""\xff""zbx_env")) return duk_error(ctx, DUK_RET_TYPE_ERROR, "cannot access internal environment"); + env = (zbx_es_env_t *)duk_to_pointer(ctx, -1); duk_pop(ctx); @@ -131,7 +139,10 @@ static duk_ret_t es_httprequest_ctor(duk_context *ctx) memset(request, 0, sizeof(zbx_es_httprequest_t)); if (NULL == (request->handle = curl_easy_init())) - return duk_error(ctx, DUK_RET_TYPE_ERROR, "cannot initialize cURL library"); + { + err_index = duk_push_error_object(ctx, DUK_RET_EVAL_ERROR, "cannot initialize cURL library"); + goto out; + } ZBX_CURL_SETOPT(ctx, request->handle, CURLOPT_COOKIEFILE, "", err); ZBX_CURL_SETOPT(ctx, request->handle, CURLOPT_FOLLOWLOCATION, 1L, err); @@ -147,6 +158,17 @@ static duk_ret_t es_httprequest_ctor(duk_context *ctx) duk_push_c_function(ctx, es_httprequest_dtor, 1); duk_set_finalizer(ctx, -2); +out: + if (-1 != err_index) + { + if (NULL != request) + { + if (NULL != request->handle) + curl_easy_cleanup(request->handle); + zbx_free(request); + } + return duk_throw(ctx); + } return 0; } @@ -163,18 +185,26 @@ static duk_ret_t es_httprequest_add_header(duk_context *ctx) zbx_es_httprequest_t *request; CURLcode err; char *utf8 = NULL; + int err_index = -1; if (NULL == (request = es_httprequest(ctx))) - return duk_error(ctx, DUK_RET_TYPE_ERROR, "internal scripting error: null object"); + return duk_error(ctx, DUK_RET_EVAL_ERROR, "internal scripting error: null object"); if (SUCCEED != zbx_cesu8_to_utf8(duk_to_string(ctx, 0), &utf8)) - return duk_error(ctx, DUK_RET_TYPE_ERROR, "cannot convert header to utf8"); + { + err_index = duk_push_error_object(ctx, DUK_RET_TYPE_ERROR, "cannot convert header to utf8"); + goto out; + } request->headers = curl_slist_append(request->headers, utf8); ZBX_CURL_SETOPT(ctx, request->handle, CURLOPT_HTTPHEADER, request->headers, err); request->custom_header = 1; +out: zbx_free(utf8); + if (-1 != err_index) + return duk_throw(ctx); + return 0; } @@ -190,7 +220,7 @@ static duk_ret_t es_httprequest_clear_header(duk_context *ctx) zbx_es_httprequest_t *request; if (NULL == (request = es_httprequest(ctx))) - return duk_error(ctx, DUK_RET_TYPE_ERROR, "internal scripting error: null object"); + return duk_error(ctx, DUK_RET_EVAL_ERROR, "internal scripting error: null object"); curl_slist_free_all(request->headers); request->headers = NULL; @@ -214,11 +244,11 @@ static duk_ret_t es_httprequest_query(duk_context *ctx, const char *http_request zbx_es_httprequest_t *request; char *url = NULL, *contents = NULL; CURLcode err; - duk_ret_t ret; + int err_index = -1; if (SUCCEED != zbx_cesu8_to_utf8(duk_to_string(ctx, 0), &url)) { - ret = duk_error(ctx, DUK_RET_TYPE_ERROR, "cannot convert URL to utf8"); + err_index = duk_push_error_object(ctx, DUK_RET_TYPE_ERROR, "cannot convert URL to utf8"); goto out; } @@ -226,14 +256,15 @@ static duk_ret_t es_httprequest_query(duk_context *ctx, const char *http_request { if (SUCCEED != zbx_cesu8_to_utf8(duk_to_string(ctx, 1), &contents)) { - ret = duk_error(ctx, DUK_RET_TYPE_ERROR, "cannot convert request contents to utf8"); + err_index = duk_push_error_object(ctx, DUK_RET_TYPE_ERROR, + "cannot convert request contents to utf8"); goto out; } } if (NULL == (request = es_httprequest(ctx))) { - ret = duk_error(ctx, DUK_RET_TYPE_ERROR, "internal scripting error: null object"); + err_index = duk_push_error_object(ctx, DUK_RET_EVAL_ERROR, "internal scripting error: null object"); goto out; } @@ -266,17 +297,20 @@ static duk_ret_t es_httprequest_query(duk_context *ctx, const char *http_request if (CURLE_OK != (err = curl_easy_perform(request->handle))) { - ret = duk_error(ctx, DUK_RET_TYPE_ERROR, "cannot get URL: %s.", curl_easy_strerror(err)); + err_index = duk_push_error_object(ctx, DUK_RET_EVAL_ERROR, "cannot get URL: %s.", + curl_easy_strerror(err)); goto out; } duk_push_string(ctx, request->data); - ret = 1; out: zbx_free(url); zbx_free(contents); - return ret; + if (-1 != err_index) + return duk_throw(ctx); + + return 1; } /****************************************************************************** @@ -338,11 +372,15 @@ static duk_ret_t es_httprequest_set_proxy(duk_context *ctx) { zbx_es_httprequest_t *request; CURLcode err; + int err_index = -1; if (NULL == (request = es_httprequest(ctx))) - return duk_error(ctx, DUK_RET_TYPE_ERROR, "internal scripting error: null object"); + return duk_error(ctx, DUK_RET_EVAL_ERROR, "internal scripting error: null object"); ZBX_CURL_SETOPT(ctx, request->handle, CURLOPT_PROXY, duk_to_string(ctx, 0), err); +out: + if (-1 != err_index) + return duk_throw(ctx); return 1; } @@ -361,10 +399,10 @@ static duk_ret_t es_httprequest_status(duk_context *ctx) CURLcode err; if (NULL == (request = es_httprequest(ctx))) - return duk_error(ctx, DUK_RET_TYPE_ERROR, "internal scripting error: null object"); + return duk_error(ctx, DUK_RET_EVAL_ERROR, "internal scripting error: null object"); if (CURLE_OK != (err = curl_easy_getinfo(request->handle, CURLINFO_RESPONSE_CODE, &response_code))) - return duk_error(ctx, DUK_RET_TYPE_ERROR, "cannot obtain request status: %s", curl_easy_strerror(err)); + return duk_error(ctx, DUK_RET_EVAL_ERROR, "cannot obtain request status: %s", curl_easy_strerror(err)); duk_push_number(ctx, (duk_double_t)response_code); @@ -388,9 +426,9 @@ static const duk_function_list_entry httprequest_methods[] = { static duk_ret_t es_httprequest_ctor(duk_context *ctx) { if (!duk_is_constructor_call(ctx)) - return DUK_RET_TYPE_ERROR; + return DUK_RET_EVAL_ERROR; - return duk_error(ctx, DUK_RET_TYPE_ERROR, "missing cURL library"); + return duk_error(ctx, DUK_RET_EVAL_ERROR, "missing cURL library"); } static const duk_function_list_entry httprequest_methods[] = { diff --git a/src/libs/zbxembed/zabbix.c b/src/libs/zbxembed/zabbix.c index ee487552df6..7b7bc8d1094 100644 --- a/src/libs/zbxembed/zabbix.c +++ b/src/libs/zbxembed/zabbix.c @@ -70,7 +70,7 @@ static duk_ret_t es_zabbix_log(duk_context *ctx) { zbx_es_env_t *env; char *message = NULL; - int level; + int level, err_index = -1; duk_memory_functions out_funcs; level = duk_to_int(ctx, 0); @@ -87,9 +87,13 @@ static duk_ret_t es_zabbix_log(duk_context *ctx) env = (zbx_es_env_t *)out_funcs.udata; if (NULL == env->json) + goto out; + + if (ZBX_ES_LOG_MEMORY_LIMIT < env->json->buffer_size) /* approximate limit */ { - zbx_free(message); - return 0; + err_index = duk_push_error_object(ctx, DUK_RET_EVAL_ERROR, "log exceeds the maximum size of " + ZBX_FS_UI64 " bytes.", ZBX_ES_LOG_MEMORY_LIMIT); + goto out; } zbx_json_addobject(env->json, NULL); @@ -97,14 +101,11 @@ static duk_ret_t es_zabbix_log(duk_context *ctx) zbx_json_adduint64(env->json, "ms", zbx_get_duration_ms(&env->start_time)); zbx_json_addstring(env->json, "message", message, ZBX_JSON_TYPE_STRING); zbx_json_close(env->json); - +out: zbx_free(message); - if (ZBX_ES_LOG_MEMORY_LIMIT < env->json->buffer_size) /* approximate limit */ - { - return duk_error(ctx, DUK_RET_TYPE_ERROR, "log exceeds the maximum size of " ZBX_FS_UI64 " bytes.", - ZBX_ES_LOG_MEMORY_LIMIT); - } + if (-1 != err_index) + return duk_throw(ctx); return 0; } diff --git a/src/zabbix_js/zabbix_js.c b/src/zabbix_js/zabbix_js.c index 03904f5c414..2bffcd13150 100644 --- a/src/zabbix_js/zabbix_js.c +++ b/src/zabbix_js/zabbix_js.c @@ -151,6 +151,7 @@ static char *execute_script(const char *script, const char *param, int timeout, if (FAIL == zbx_es_init_env(&es, &errmsg)) { *error = zbx_dsprintf(NULL, "cannot initialize scripting environment: %s", errmsg); + zbx_free(errmsg); return NULL; } @@ -160,12 +161,14 @@ static char *execute_script(const char *script, const char *param, int timeout, if (FAIL == zbx_es_compile(&es, script, &code, &size, &errmsg)) { *error = zbx_dsprintf(NULL, "cannot compile script: %s", errmsg); + zbx_free(errmsg); goto out; } if (FAIL == zbx_es_execute(&es, script, code, size, param, &result, &errmsg)) { *error = zbx_dsprintf(NULL, "cannot execute script: %s", errmsg); + zbx_free(errmsg); goto out; } out: |