From 19424515ec8b3dcb133cef30bf10f8c6b19cc3aa Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Sun, 19 Sep 2021 12:08:48 +0100 Subject: [PATCH] [Minor] Fix some leaks on error paths Found by: coverity scan --- src/lua/lua_config.c | 11 ++++---- src/lua/lua_cryptobox.c | 56 ++++++++++++++++++++++++++++++++------ src/lua/lua_dns_resolver.c | 12 ++++---- src/lua/lua_html.cxx | 2 +- src/lua/lua_http.c | 26 +++++++++++------- src/lua/lua_kann.c | 10 +++++-- src/lua/lua_redis.c | 2 ++ src/lua/lua_rsa.c | 6 ++-- src/lua/lua_task.c | 32 +++++++++++++--------- src/lua/lua_tcp.c | 7 +++-- src/lua/lua_text.c | 12 ++++---- 11 files changed, 121 insertions(+), 55 deletions(-) diff --git a/src/lua/lua_config.c b/src/lua/lua_config.c index bfacb2be8..8b0a4a46c 100644 --- a/src/lua/lua_config.c +++ b/src/lua/lua_config.c @@ -1614,9 +1614,9 @@ rspamd_register_symbol_fromlua (lua_State *L, rspamd_symcache_set_allowed_settings_ids (cfg->cache, name, ids, nids); - - g_free (ids); } + + g_free (ids); } if (forbidden_ids) { @@ -1636,9 +1636,9 @@ rspamd_register_symbol_fromlua (lua_State *L, rspamd_symcache_set_forbidden_settings_ids (cfg->cache, name, ids, nids); - - g_free (ids); } + + g_free (ids); } return ret; @@ -4417,9 +4417,10 @@ lua_config_init_subsystem (lua_State *L) rspamd_symcache_init (cfg->cache); } else { + int ret = luaL_error (L, "invalid param: %s", parts[i]); g_strfreev (parts); - return luaL_error (L, "invalid param: %s", parts[i]); + return ret; } } diff --git a/src/lua/lua_cryptobox.c b/src/lua/lua_cryptobox.c index f712fad0c..9cea18311 100644 --- a/src/lua/lua_cryptobox.c +++ b/src/lua/lua_cryptobox.c @@ -706,6 +706,9 @@ lua_cryptobox_signature_load (lua_State *L) alg = RSPAMD_CRYPTOBOX_MODE_25519; } else { + munmap (data, st.st_size); + close (fd); + return luaL_error (L, "invalid keypair algorithm: %s", str); } } @@ -1192,6 +1195,7 @@ lua_cryptobox_hash_create (lua_State *L) t = lua_check_text (L, 1); if (!t) { + REF_RELEASE (h); return luaL_error (L, "invalid arguments"); } @@ -1243,6 +1247,7 @@ lua_cryptobox_hash_create_specific (lua_State *L) t = lua_check_text (L, 2); if (!t) { + REF_RELEASE (h); return luaL_error (L, "invalid arguments"); } @@ -1289,6 +1294,7 @@ lua_cryptobox_hash_create_keyed (lua_State *L) t = lua_check_text (L, 2); if (!t) { + REF_RELEASE (h); return luaL_error (L, "invalid arguments"); } @@ -1332,6 +1338,10 @@ lua_cryptobox_hash_create_specific_keyed (lua_State *L) if (key != NULL && type != NULL) { h = rspamd_lua_hash_create (type, key, keylen); + if (h == NULL) { + return luaL_error (L, "invalid hash type: %s", type); + } + if (lua_type (L, 3) == LUA_TSTRING) { s = lua_tolstring (L, 3, &len); } @@ -1339,6 +1349,8 @@ lua_cryptobox_hash_create_specific_keyed (lua_State *L) t = lua_check_text (L, 3); if (!t) { + REF_RELEASE (h); + return luaL_error (L, "invalid arguments"); } @@ -1962,6 +1974,7 @@ lua_cryptobox_encrypt_memory (lua_State *L) struct rspamd_lua_text *t, *res; gsize len = 0, outlen = 0; GError *err = NULL; + bool owned_pk = false; if (lua_type (L, 1) == LUA_TUSERDATA) { if (rspamd_lua_check_udata_maybe (L, 1, "rspamd{cryptobox_keypair}")) { @@ -1979,13 +1992,14 @@ lua_cryptobox_encrypt_memory (lua_State *L) pk = rspamd_pubkey_from_base32 (b32, blen, RSPAMD_KEYPAIR_KEX, lua_toboolean (L, 3) ? RSPAMD_CRYPTOBOX_MODE_NIST : RSPAMD_CRYPTOBOX_MODE_25519); + owned_pk = true; } if (lua_isuserdata (L, 2)) { t = lua_check_text (L, 2); if (!t) { - return luaL_error (L, "invalid arguments"); + goto err; } data = t->start; @@ -1997,7 +2011,7 @@ lua_cryptobox_encrypt_memory (lua_State *L) if (!(kp || pk) || !data) { - return luaL_error (L, "invalid arguments"); + goto err; } if (kp) { @@ -2008,7 +2022,7 @@ lua_cryptobox_encrypt_memory (lua_State *L) return ret; } } - else if (pk) { + else { if (!rspamd_pubkey_encrypt (pk, data, len, &out, &outlen, &err)) { gint ret = luaL_error (L, "cannot encrypt data: %s", err->message); g_error_free (err); @@ -2023,7 +2037,18 @@ lua_cryptobox_encrypt_memory (lua_State *L) res->len = outlen; rspamd_lua_setclass (L, "rspamd{text}", -1); + if (owned_pk) { + rspamd_pubkey_unref (pk); + } + return 1; +err: + + if (owned_pk) { + rspamd_pubkey_unref (pk); + } + + return luaL_error (L, "invalid arguments"); } /*** @@ -2045,6 +2070,7 @@ lua_cryptobox_encrypt_file (lua_State *L) struct rspamd_lua_text *res; gsize len = 0, outlen = 0; GError *err = NULL; + bool own_pk = false; if (lua_type (L, 1) == LUA_TUSERDATA) { if (rspamd_lua_check_udata_maybe (L, 1, "rspamd{cryptobox_keypair}")) { @@ -2062,13 +2088,14 @@ lua_cryptobox_encrypt_file (lua_State *L) pk = rspamd_pubkey_from_base32 (b32, blen, RSPAMD_KEYPAIR_KEX, lua_toboolean (L, 3) ? RSPAMD_CRYPTOBOX_MODE_NIST : RSPAMD_CRYPTOBOX_MODE_25519); + own_pk = true; } filename = luaL_checkstring (L, 2); data = rspamd_file_xmap (filename, PROT_READ, &len, TRUE); if (!(kp || pk) || !data) { - return luaL_error (L, "invalid arguments"); + goto err; } if (kp) { @@ -2098,8 +2125,17 @@ lua_cryptobox_encrypt_file (lua_State *L) res->len = outlen; rspamd_lua_setclass (L, "rspamd{text}", -1); munmap (data, len); + if (own_pk) { + rspamd_pubkey_unref (pk); + } return 1; + +err: + if (own_pk) { + rspamd_pubkey_unref (pk); + } + return luaL_error (L, "invalid arguments"); } /*** @@ -2178,12 +2214,15 @@ lua_cryptobox_decrypt_file (lua_State *L) GError *err = NULL; kp = lua_check_cryptobox_keypair (L, 1); + if (!kp) { + return luaL_error (L, "invalid arguments; keypair is expected"); + } + filename = luaL_checkstring (L, 2); data = rspamd_file_xmap (filename, PROT_READ, &len, TRUE); - - - if (!kp || !data) { - return luaL_error (L, "invalid arguments"); + if (!data) { + return luaL_error (L, "invalid arguments; cannot mmap %s: %s", + filename, strerror(errno)); } if (!rspamd_keypair_decrypt (kp, data, len, &out, &outlen, &err)) { @@ -2446,6 +2485,7 @@ lua_cryptobox_pbkdf (lua_State *L) if (pwlen == 0) { lua_pushnil (L); + g_free (password); return 1; } diff --git a/src/lua/lua_dns_resolver.c b/src/lua/lua_dns_resolver.c index 7eaa54da6..f43267dc6 100644 --- a/src/lua/lua_dns_resolver.c +++ b/src/lua/lua_dns_resolver.c @@ -504,16 +504,18 @@ lua_dns_resolver_resolve_common (lua_State *L, } return 1; + err: + /* Callback is not called in this case */ + if (cbdata->cbref != -1) { + luaL_unref (L, LUA_REGISTRYINDEX, cbdata->cbref); + } + if (!pool) { /* Free resources */ g_free (cbdata->to_resolve); g_free (cbdata->user_str); - } - - /* Callback is not called in this case */ - if (cbdata->cbref != -1) { - luaL_unref (L, LUA_REGISTRYINDEX, cbdata->cbref); + g_free (cbdata); } lua_pushnil (L); diff --git a/src/lua/lua_html.cxx b/src/lua/lua_html.cxx index b47a110d4..666b08a60 100644 --- a/src/lua/lua_html.cxx +++ b/src/lua/lua_html.cxx @@ -565,7 +565,7 @@ lua_html_tag_get_flags (lua_State *L) struct lua_html_tag *ltag = lua_check_html_tag (L, 1); gint i = 1; - if (ltag->tag) { + if (ltag && ltag->tag) { /* Push flags */ lua_createtable (L, 4, 0); if (ltag->tag->flags & FL_HREF) { diff --git a/src/lua/lua_http.c b/src/lua/lua_http.c index a92c1fe8f..68c9bb927 100644 --- a/src/lua/lua_http.c +++ b/src/lua/lua_http.c @@ -441,15 +441,15 @@ lua_http_make_connection (struct lua_http_cbdata *cbd) if (cbd->task) { cbd->conn->log_tag = cbd->task->task_pool->tag.uid; + + if (cbd->item) { + rspamd_symcache_item_async_inc (cbd->task, cbd->item, M); + } } else if (cbd->cfg) { cbd->conn->log_tag = cbd->cfg->cfg_pool->tag.uid; } - if (cbd->item) { - rspamd_symcache_item_async_inc (cbd->task, cbd->item, M); - } - struct rspamd_http_message *msg = cbd->msg; /* Message is now owned by a connection object */ @@ -652,10 +652,13 @@ lua_http_request (lua_State *L) if (lua_type (L, -1) == LUA_TUSERDATA) { task = lua_check_task (L, -1); - ev_base = task->event_loop; - resolver = task->resolver; - session = task->s; - cfg = task->cfg; + + if (task) { + ev_base = task->event_loop; + resolver = task->resolver; + session = task->s; + cfg = task->cfg; + } } lua_pop (L, 1); @@ -823,7 +826,10 @@ lua_http_request (lua_State *L) } else { t = lua_check_text (L, -1); - body = rspamd_fstring_append (body, t->start, t->len); + + if (t) { + body = rspamd_fstring_append(body, t->start, t->len); + } } lua_pop (L, 1); @@ -1043,7 +1049,7 @@ lua_http_request (lua_State *L) cbd->session = session; } - if (rspamd_parse_inet_address (&cbd->addr, + if (msg->host && rspamd_parse_inet_address (&cbd->addr, msg->host->str, msg->host->len, RSPAMD_INET_ADDRESS_PARSE_DEFAULT)) { /* Host is numeric IP, no need to resolve */ gboolean ret; diff --git a/src/lua/lua_kann.c b/src/lua/lua_kann.c index b5ddc55f2..9e37f3b0a 100644 --- a/src/lua/lua_kann.c +++ b/src/lua/lua_kann.c @@ -803,11 +803,11 @@ lua_kann_new_weight_conv1d (lua_State *L) static int lua_kann_new_leaf (lua_State *L) { - gint dim = luaL_checkinteger (L, 1), i, *ar; + int dim = luaL_checkinteger (L, 1), i, *ar; kad_node_t *t; if (dim >= 1 && dim < KAD_MAX_DIM && lua_istable (L, 2)) { - ar = g_malloc0 (sizeof (ar) * dim); + ar = g_new0 (int, dim); for (i = 0; i < dim; i ++) { lua_rawgeti (L, 2, i + 1); @@ -962,6 +962,10 @@ lua_kann_load (lua_State *L) t = lua_check_text (L, 1); + if (!t) { + return luaL_error (L, "invalid arguments"); + } + #ifndef HAVE_FMEMOPEN return luaL_error (L, "no support of loading from memory on your system"); #endif @@ -1043,7 +1047,7 @@ lua_kann_train1 (lua_State *L) } if (n_out <= 0) { - return luaL_error (L, "invalid outputs count: %d", n_in); + return luaL_error (L, "invalid outputs count: %d", n_out); } if (n != rspamd_lua_table_size (L, 3) || n == 0) { diff --git a/src/lua/lua_redis.c b/src/lua/lua_redis.c index 057a21c95..6085f6328 100644 --- a/src/lua/lua_redis.c +++ b/src/lua/lua_redis.c @@ -1348,6 +1348,8 @@ lua_redis_make_request_sync (lua_State *L) rspamd_inet_address_free (ip); } msg_err ("bad arguments for redis request"); + lua_redis_free_args (args, arglens, nargs); + lua_pushboolean (L, FALSE); } diff --git a/src/lua/lua_rsa.c b/src/lua/lua_rsa.c index 5e34bc2c6..0d4a268ed 100644 --- a/src/lua/lua_rsa.c +++ b/src/lua/lua_rsa.c @@ -450,7 +450,6 @@ lua_rsa_signature_load (lua_State *L) lua_pushnil (L); } else { - sig = g_malloc (sizeof (rspamd_fstring_t)); if (fstat (fd, &st) == -1 || (data = mmap (NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0)) @@ -660,8 +659,10 @@ lua_rsa_sign_memory (lua_State *L) if (rsa != NULL && data != NULL) { signature = rspamd_fstring_sized_new (RSA_size (rsa)); + + guint siglen = signature->len; ret = RSA_sign (NID_sha256, data, sz, - signature->str, (guint *)&signature->len, rsa); + signature->str, &siglen, rsa); if (ret != 1) { rspamd_fstring_free (signature); @@ -670,6 +671,7 @@ lua_rsa_sign_memory (lua_State *L) ERR_error_string (ERR_get_error (), NULL)); } else { + signature->len = siglen; psig = lua_newuserdata (L, sizeof (rspamd_fstring_t *)); rspamd_lua_setclass (L, "rspamd{rsa_signature}", -1); *psig = signature; diff --git a/src/lua/lua_task.c b/src/lua/lua_task.c index 393a370a8..589d45439 100644 --- a/src/lua/lua_task.c +++ b/src/lua/lua_task.c @@ -5059,22 +5059,28 @@ lua_task_get_symbols_tokens (lua_State *L) struct rspamd_task *task = lua_check_task (L, 1); struct tokens_foreach_cbdata cbd; - cbd.task = task; - cbd.L = L; - cbd.idx = 1; - cbd.normalize = TRUE; + if (task) { + cbd.task = task; + cbd.L = L; + cbd.idx = 1; + cbd.normalize = TRUE; - if (lua_type (L, 2) == LUA_TBOOLEAN) { - cbd.normalize = lua_toboolean (L, 2); + if (lua_type(L, 2) == LUA_TBOOLEAN) { + cbd.normalize = lua_toboolean(L, 2); + } + else { + cbd.normalize = TRUE; + } + + lua_createtable(L, + rspamd_symcache_stats_symbols_count(task->cfg->cache), 0); + rspamd_symcache_foreach(task->cfg->cache, tokens_foreach_cb, &cbd); } else { - cbd.normalize = TRUE; + return luaL_error (L, "invalid arguments"); } - lua_createtable (L, - rspamd_symcache_stats_symbols_count (task->cfg->cache), 0); - rspamd_symcache_foreach (task->cfg->cache, tokens_foreach_cb, &cbd); - + /* Return type is table created */ return 1; } @@ -6058,7 +6064,7 @@ lua_task_store_in_file (lua_State *L) gboolean force_new = FALSE, keep = FALSE; gchar fpath[PATH_MAX]; const gchar *tmpmask = NULL, *fname = NULL; - guint64 mode = 00600; + guint mode = 00600; gint fd; struct lua_file_cbdata *cbdata; GError *err = NULL; @@ -6097,7 +6103,7 @@ lua_task_store_in_file (lua_State *L) rspamd_snprintf (fpath, sizeof (fpath), "%s", tmpmask); } - fd = mkstemp (fpath); + fd = g_mkstemp_full (fpath, O_WRONLY|O_CREAT|O_EXCL, mode); fname = fpath; if (fd != -1) { diff --git a/src/lua/lua_tcp.c b/src/lua/lua_tcp.c index 5a34475bc..a1c1f0b20 100644 --- a/src/lua/lua_tcp.c +++ b/src/lua/lua_tcp.c @@ -785,6 +785,7 @@ lua_tcp_write_helper (struct lua_tcp_cbdata *cbd) struct iovec *start; guint niov, i; gint flags = 0; + bool allocated_iov = false; gsize remain; gssize r; struct iovec *cur_iov; @@ -811,6 +812,7 @@ lua_tcp_write_helper (struct lua_tcp_cbdata *cbd) } else { cur_iov = g_malloc0 (niov * sizeof (struct iovec)); + allocated_iov = true; } memcpy (cur_iov, wh->iov, niov * sizeof (struct iovec)); @@ -848,7 +850,7 @@ lua_tcp_write_helper (struct lua_tcp_cbdata *cbd) r = sendmsg (cbd->fd, &msg, flags); } - if (niov >= 1024) { + if (allocated_iov) { g_free (cur_iov); } @@ -1242,7 +1244,7 @@ lua_tcp_register_event (struct lua_tcp_cbdata *cbd) static void lua_tcp_register_watcher (struct lua_tcp_cbdata *cbd) { - if (cbd->item) { + if (cbd->item && cbd->task) { rspamd_symcache_item_async_inc (cbd->task, cbd->item, M); } } @@ -1690,6 +1692,7 @@ lua_tcp_request (lua_State *L) } if (resolver == NULL && cfg == NULL && task == NULL) { + g_free (cbd); return luaL_error (L, "tcp request has bad params: one of " "{resolver,task,config} should be set"); } diff --git a/src/lua/lua_text.c b/src/lua/lua_text.c index bec16c3b6..afda7d941 100644 --- a/src/lua/lua_text.c +++ b/src/lua/lua_text.c @@ -1519,7 +1519,7 @@ lua_text_oneline (lua_State *L) struct rspamd_lua_text *t = lua_check_text (L, 1); const gchar *p, *end; gchar *dest, *d; - gsize byteset[32 / sizeof(gsize)]; /* Bitset for ascii */ + guint64 byteset[32 / sizeof(guint64)]; /* Bitset for ascii */ gboolean copy = TRUE, seen_8bit = FALSE; guint *plen; @@ -1553,14 +1553,14 @@ lua_text_oneline (lua_State *L) /* Fill pattern bitset */ memset (byteset, 0, sizeof byteset); /* All spaces */ - byteset[0] |= GSIZE_FROM_LE (0x100003600); + byteset[0] |= GUINT64_FROM_LE (0x100003600LLU); /* Control characters */ - byteset[0] |= GSIZE_FROM_LE (0xffffffff); + byteset[0] |= GUINT64_FROM_LE (0xffffffffLLU); /* Del character */ - byteset[1] |= GSIZE_FROM_LE (0x8000000000000000); + byteset[1] |= GUINT64_FROM_LE (0x8000000000000000LLU); /* 8 bit characters */ - byteset[2] |= GSIZE_FROM_LE (0xffffffffffffffffLLU); - byteset[3] |= GSIZE_FROM_LE (0xffffffffffffffffLLU); + byteset[2] |= GUINT64_FROM_LE (0xffffffffffffffffLLU); + byteset[3] |= GUINT64_FROM_LE (0xffffffffffffffffLLU); p = t->start; end = t->start + t->len; -- 2.39.5