diff options
author | Andris Zeila <andris.zeila@zabbix.com> | 2020-06-26 09:44:41 +0300 |
---|---|---|
committer | Andris Zeila <andris.zeila@zabbix.com> | 2020-06-26 09:44:59 +0300 |
commit | c5ce5256a3d9f9361b122407f51e2bacf27e4ca7 (patch) | |
tree | 07507e74a664799ca9d080447e681265f10c501b /src | |
parent | 454ed551c43c8695a3213f63b1984908f069861c (diff) |
.......PS. [ZBX-17837] fixed memory leaks when handling CurlHttpRequest errors
* commit '7c735c2d13a8da7f149295b0de67fcffaebf7e5d':
.D........ [ZBX-17837] added changelog entry
.......PS. [ZBX-17837] moved check for Zabbix.Log json buffer limits before adding log to the buffer. Otherwise it would be possible to exceed the limits if user would handle exception generated by Zabbix.Log() limits.
.......PS. [ZBX-17837] fixed error handling in CurlHttpRequest implementation
.......... [ZBX-17837] reset error message after embeded scripting api call failure
.......PS. [ZBX-17837] fixed memory leaks if ZBX_CURL_SETOPT() fails
.......PS. [ZBX-17837] modified macro ZBX_CURL_SETOPT() to avoid throwing duk_error()
(cherry picked from commit a807da8fdeff9d2edd851f942c84fdb7c811e813)
Diffstat (limited to 'src')
-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 |
3 files changed, 69 insertions, 27 deletions
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: |