diff --git a/scripts/lua/modules/alert_store/alert_store.lua b/scripts/lua/modules/alert_store/alert_store.lua index ece68ab350..8b3f4aaa17 100644 --- a/scripts/lua/modules/alert_store/alert_store.lua +++ b/scripts/lua/modules/alert_store/alert_store.lua @@ -415,6 +415,13 @@ function alert_store:build_sql_cond(cond, is_write) sql_val = snmp_info[2] end + -- Reject non-numeric interface IDs to prevent SQL injection + local num_val = tonumber(sql_val) + if not num_val then + return "(1 = 0)" + end + sql_val = tostring(math.floor(num_val)) + if self._alert_entity == alert_entities.snmp_device then -- snmp entity sql_cond = self:get_column_name('port', is_write) .. sql_op .. sql_val @@ -514,17 +521,18 @@ function alert_store:build_sql_cond(cond, is_write) -- String else + local escaped_val = self:_escape(cond.value) if cond.op == 'in' then - sql_cond = real_field .. ' LIKE ' .. string.format("'%%%s%%'", cond.value) + sql_cond = real_field .. ' LIKE ' .. string.format("'%%%s%%'", escaped_val) elseif cond.op == 'nin' then - sql_cond = real_field .. ' NOT LIKE ' .. string.format("'%%%s%%'", cond.value) + sql_cond = real_field .. ' NOT LIKE ' .. string.format("'%%%s%%'", escaped_val) elseif cond.op == 'empty' then sql_cond = real_field .. ' = ' .. "''" elseif cond.op == 'nempty' then sql_cond = real_field .. ' <> ' .. "''" else -- Any other operator - sql_cond = string.format("%s %s ('%s')", real_field, sql_op, cond.value) + sql_cond = string.format("%s %s ('%s')", real_field, sql_op, escaped_val) end end diff --git a/scripts/lua/modules/vulnerability_scan/vs_utils.lua b/scripts/lua/modules/vulnerability_scan/vs_utils.lua index 4d9cf9f5c0..d671734a4d 100644 --- a/scripts/lua/modules/vulnerability_scan/vs_utils.lua +++ b/scripts/lua/modules/vulnerability_scan/vs_utils.lua @@ -2687,6 +2687,15 @@ function vs_utils.runCommand(scan_command, use_coroutines) return result end +-- ********************************************************** +-- Validates that a nmap target is a plain IP address or CIDR (IPv4 or IPv6). +-- Rejects anything containing shell metacharacters to prevent command injection. +local function is_valid_nmap_target(target) + if isEmptyString(target) then return false end + local ip = target:match("^([^/]+)") or target + return isIPv4(ip) or isIPv6(ip) +end + -- ********************************************************** -- Function to scan host function vs_utils.nmap_scan_host(command, host_ip, ports, use_coroutines, module_name) @@ -2696,6 +2705,11 @@ function vs_utils.nmap_scan_host(command, host_ip, ports, use_coroutines, module return nil end + if not is_valid_nmap_target(host_ip) then + traceError(TRACE_WARNING, TRACE_CONSOLE, "nmap_scan_host: invalid target rejected: " .. tostring(host_ip)) + return nil + end + -- IPv6 check if(string.contains(host_ip, ':')) then command = command .. " -6 " end @@ -2722,6 +2736,11 @@ end -- ********************************************************** -- Function to check if a host is available function vs_utils.nmap_check_host(host_ip, use_coroutines) + if not is_valid_nmap_target(host_ip) then + traceError(TRACE_WARNING, TRACE_CONSOLE, "nmap_check_host: invalid target rejected: " .. tostring(host_ip)) + return nil, 0 + end + local nmap = vs_utils.get_nmap_path() local scan_command = nmap.." -sn" diff --git a/scripts/lua/rest/v2/add/host/to_scan.lua b/scripts/lua/rest/v2/add/host/to_scan.lua index 9b533d1491..fc5454dd5f 100644 --- a/scripts/lua/rest/v2/add/host/to_scan.lua +++ b/scripts/lua/rest/v2/add/host/to_scan.lua @@ -23,6 +23,14 @@ if isEmptyString(host) or isEmptyString(scan_type) then rest_utils.answer(rest_utils.consts.err.bad_content) return end + +-- Validate that host is a plain IP address or CIDR before passing to nmap, +-- preventing OS command injection via shell metacharacters in the host parameter. +local ip_part = host:match("^([^/]+)") or host +if not isIPv4(ip_part) and not isIPv6(ip_part) then + rest_utils.answer(rest_utils.consts.err.bad_content) + return +end local result = nil local id = nil diff --git a/src/Ntop.cpp b/src/Ntop.cpp index db313d691a..e625b201cb 100644 --- a/src/Ntop.cpp +++ b/src/Ntop.cpp @@ -1757,23 +1757,38 @@ bool Ntop::checkHTTPAuth(const char* user, const char* password, if (!password || !password[0]) return false; - postLen = 100 + strlen(user) + strlen(password); - if (!(httpUrl = (char*)calloc(sizeof(char), MAX_HTTP_AUTHENTICATOR_LEN)) || - !(postData = (char*)calloc(sizeof(char), postLen + 1)) || - !(returnData = (char*)calloc( - sizeof(char), MAX_HTTP_AUTHENTICATOR_RETURN_DATA_LEN + 1))) { - ntop->getTrace()->traceEvent(TRACE_ERROR, - "HTTP: unable to allocate memory"); - goto http_auth_out; + /* JSON-escape user and password to prevent injection via '"' or '\' in credentials. + user is bounded to 32 chars and password to 129, so 2x sizes cover worst case. */ + { + char escaped_user[65] = {}, escaped_password[259] = {}; + auto json_escape = [](const char* src, char* dst, size_t dst_size) { + size_t oi = 0; + for (size_t i = 0; src[i] && oi < dst_size - 2; i++) { + if (src[i] == '"' || src[i] == '\\') dst[oi++] = '\\'; + dst[oi++] = src[i]; + } + dst[oi] = '\0'; + }; + json_escape(user, escaped_user, sizeof(escaped_user)); + json_escape(password, escaped_password, sizeof(escaped_password)); + postLen = 100 + (int)strlen(escaped_user) + (int)strlen(escaped_password); + if (!(httpUrl = (char*)calloc(sizeof(char), MAX_HTTP_AUTHENTICATOR_LEN)) || + !(postData = (char*)calloc(sizeof(char), postLen + 1)) || + !(returnData = (char*)calloc( + sizeof(char), MAX_HTTP_AUTHENTICATOR_RETURN_DATA_LEN + 1))) { + ntop->getTrace()->traceEvent(TRACE_ERROR, + "HTTP: unable to allocate memory"); + goto http_auth_out; + } + ntop->getRedis()->get((char*)PREF_HTTP_AUTHENTICATOR_URL, httpUrl, + MAX_HTTP_AUTHENTICATOR_LEN); + if (!httpUrl[0]) { + ntop->getTrace()->traceEvent(TRACE_ERROR, "HTTP: no http url set !"); + goto http_auth_out; + } + snprintf(postData, postLen, "{\"user\": \"%s\", \"password\": \"%s\"}", + escaped_user, escaped_password); } - ntop->getRedis()->get((char*)PREF_HTTP_AUTHENTICATOR_URL, httpUrl, - MAX_HTTP_AUTHENTICATOR_LEN); - if (!httpUrl[0]) { - ntop->getTrace()->traceEvent(TRACE_ERROR, "HTTP: no http url set !"); - goto http_auth_out; - } - snprintf(postData, postLen, "{\"user\": \"%s\", \"password\": \"%s\"}", user, - password); if (Utils::postHTTPJsonData(NULL, // no token NULL, // no digest user diff --git a/src/ZMQCollectorInterface.cpp b/src/ZMQCollectorInterface.cpp index 49b42204d9..33e2e2daf5 100644 --- a/src/ZMQCollectorInterface.cpp +++ b/src/ZMQCollectorInterface.cpp @@ -507,10 +507,22 @@ void ZMQCollectorInterface::collect_flows() { uLen = uncompressed_len = ndpi_min(ndpi_max(10 * size, MAX_ZMQ_FLOW_BUF), MAX_ZMQ_FLOW_BUF / 3); /* Compatibility mode */ - else + else { + /* Guard against integer overflow: a malicious sender could set + uncompressed_size to UINT32_MAX causing +16 to wrap to ~15, + then malloc(16) would be followed by a multi-GB uncompress write. */ + if (received_uncompressed_size > MAX_ZMQ_FLOW_BUF) { + ntop->getTrace()->traceEvent( + TRACE_WARNING, + "ZMQ: dropping message with oversized uncompressed_size claim " + "[%u > %u]", + received_uncompressed_size, MAX_ZMQ_FLOW_BUF); + continue; + } uLen = uncompressed_len = received_uncompressed_size + 16; /* We know already the uncompressed size */ + } uncompressed = (char*)malloc(uncompressed_len + 1); diff --git a/third-party/clickhouse-cpp b/third-party/clickhouse-cpp index 506248dd04..620ec41bff 160000 --- a/third-party/clickhouse-cpp +++ b/third-party/clickhouse-cpp @@ -1 +1 @@ -Subproject commit 506248dd049318226805b99ddaf873008762bef4 +Subproject commit 620ec41bff2fcaf01c4e72eb64fe47302a41b781