From 44356d875017161d3e8fe3de31351c6b8362686c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 31 Dec 2025 07:11:57 +0000 Subject: [PATCH 1/6] Initial plan From f711bf38d2fe009dd55f4a659524e6cfa616dce4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 31 Dec 2025 07:19:53 +0000 Subject: [PATCH 2/6] fix: Enhanced SQL injection protection in funcspec - Added sanitization for path parameters in mergePathParams - Added sanitization for query parameters with p- prefix in mergeQueryParams - Added sanitization for header parameters in mergeHeaderParams - Fixed IN clause to sanitize all values individually - Improved ValidSQL function with better escaping and more injection patterns - Added backslash escaping to colvalue mode - Extended dangerous keyword list in select mode Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com> --- pkg/funcspec/function_api.go | 43 ++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/pkg/funcspec/function_api.go b/pkg/funcspec/function_api.go index 5577cae..82a41df 100644 --- a/pkg/funcspec/function_api.go +++ b/pkg/funcspec/function_api.go @@ -662,7 +662,10 @@ func (h *Handler) mergePathParams(r *http.Request, sqlquery string, variables ma for k, v := range pathVars { kword := fmt.Sprintf("[%s]", k) if strings.Contains(sqlquery, kword) { - sqlquery = strings.ReplaceAll(sqlquery, kword, fmt.Sprintf("%v", v)) + // Sanitize the value before replacing + vStr := fmt.Sprintf("%v", v) + sanitized := ValidSQL(vStr, "colvalue") + sqlquery = strings.ReplaceAll(sqlquery, kword, sanitized) } variables[k] = v @@ -690,7 +693,9 @@ func (h *Handler) mergeQueryParams(r *http.Request, sqlquery string, variables m // Replace in SQL if placeholder exists if strings.Contains(sqlquery, kword) && len(val) > 0 { if strings.HasPrefix(parmk, "p-") { - sqlquery = strings.ReplaceAll(sqlquery, kword, val) + // Sanitize the parameter value before replacing + sanitized := ValidSQL(val, "colvalue") + sqlquery = strings.ReplaceAll(sqlquery, kword, sanitized) } } @@ -702,7 +707,12 @@ func (h *Handler) mergeQueryParams(r *http.Request, sqlquery string, variables m // Apply filters if allowed if allowFilter && len(parmk) > 1 && strings.Contains(strings.ToLower(sqlquery), strings.ToLower(parmk)) { if len(parmv) > 1 { - sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s IN (%s)", ValidSQL(parmk, "colname"), strings.Join(parmv, ","))) + // Sanitize each value in the IN clause + sanitizedValues := make([]string, len(parmv)) + for i, v := range parmv { + sanitizedValues[i] = ValidSQL(v, "colvalue") + } + sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s IN (%s)", ValidSQL(parmk, "colname"), strings.Join(sanitizedValues, ","))) } else { if strings.Contains(val, "match=") { colval := strings.ReplaceAll(val, "match=", "") @@ -743,7 +753,9 @@ func (h *Handler) mergeHeaderParams(r *http.Request, sqlquery string, variables kword := fmt.Sprintf("[%s]", k) if strings.Contains(sqlquery, kword) { - sqlquery = strings.ReplaceAll(sqlquery, kword, val) + // Sanitize the header value before replacing + sanitized := ValidSQL(val, "colvalue") + sqlquery = strings.ReplaceAll(sqlquery, kword, sanitized) } // Handle special headers @@ -862,17 +874,30 @@ func ValidSQL(input, mode string) string { reg := regexp.MustCompile(`[^a-zA-Z0-9_\.]`) return reg.ReplaceAllString(input, "") case "colvalue": - // For column values, escape single quotes - return strings.ReplaceAll(input, "'", "''") + // For column values, escape single quotes and backslashes + result := strings.ReplaceAll(input, "\\", "\\\\") + result = strings.ReplaceAll(result, "'", "''") + return result case "select": // For SELECT clauses, be more permissive but still safe // Remove semicolons and common SQL injection patterns - dangerous := []string{";", "--", "/*", "*/", "xp_", "sp_", "DROP ", "DELETE ", "TRUNCATE ", "UPDATE ", "INSERT "} + dangerous := []string{ + ";", "--", "/*", "*/", "xp_", "sp_", + "DROP ", "drop ", "Drop ", + "DELETE ", "delete ", "Delete ", + "TRUNCATE ", "truncate ", "Truncate ", + "UPDATE ", "update ", "Update ", + "INSERT ", "insert ", "Insert ", + "EXEC ", "exec ", "Exec ", + "EXECUTE ", "execute ", "Execute ", + "UNION ", "union ", "Union ", + "DECLARE ", "declare ", "Declare ", + "ALTER ", "alter ", "Alter ", + "CREATE ", "create ", "Create ", + } result := input for _, d := range dangerous { result = strings.ReplaceAll(result, d, "") - result = strings.ReplaceAll(result, strings.ToLower(d), "") - result = strings.ReplaceAll(result, strings.ToUpper(d), "") } return result default: From 6528e94297df984ea79ab0980607bbdf424a6cdb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 31 Dec 2025 07:20:56 +0000 Subject: [PATCH 3/6] fix: Improved SQL injection protections based on code review - Fixed backslash escaping order in colvalue mode - Added proper quoting for IN clause values - Simplified dangerous pattern matching with case-insensitive approach - All funcspec tests pass (except pre-existing test failure) Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com> --- pkg/funcspec/function_api.go | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/pkg/funcspec/function_api.go b/pkg/funcspec/function_api.go index 82a41df..b8469c1 100644 --- a/pkg/funcspec/function_api.go +++ b/pkg/funcspec/function_api.go @@ -707,10 +707,12 @@ func (h *Handler) mergeQueryParams(r *http.Request, sqlquery string, variables m // Apply filters if allowed if allowFilter && len(parmk) > 1 && strings.Contains(strings.ToLower(sqlquery), strings.ToLower(parmk)) { if len(parmv) > 1 { - // Sanitize each value in the IN clause + // Sanitize each value in the IN clause and wrap in quotes sanitizedValues := make([]string, len(parmv)) for i, v := range parmv { - sanitizedValues[i] = ValidSQL(v, "colvalue") + sanitized := ValidSQL(v, "colvalue") + // Wrap each value in single quotes for SQL IN clause + sanitizedValues[i] = fmt.Sprintf("'%s'", sanitized) } sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s IN (%s)", ValidSQL(parmk, "colname"), strings.Join(sanitizedValues, ","))) } else { @@ -875,29 +877,31 @@ func ValidSQL(input, mode string) string { return reg.ReplaceAllString(input, "") case "colvalue": // For column values, escape single quotes and backslashes + // Note: Backslashes must be escaped first, then single quotes result := strings.ReplaceAll(input, "\\", "\\\\") result = strings.ReplaceAll(result, "'", "''") return result case "select": // For SELECT clauses, be more permissive but still safe - // Remove semicolons and common SQL injection patterns + // Remove semicolons and common SQL injection patterns (case-insensitive) dangerous := []string{ ";", "--", "/*", "*/", "xp_", "sp_", - "DROP ", "drop ", "Drop ", - "DELETE ", "delete ", "Delete ", - "TRUNCATE ", "truncate ", "Truncate ", - "UPDATE ", "update ", "Update ", - "INSERT ", "insert ", "Insert ", - "EXEC ", "exec ", "Exec ", - "EXECUTE ", "execute ", "Execute ", - "UNION ", "union ", "Union ", - "DECLARE ", "declare ", "Declare ", - "ALTER ", "alter ", "Alter ", - "CREATE ", "create ", "Create ", + "drop ", "delete ", "truncate ", "update ", "insert ", + "exec ", "execute ", "union ", "declare ", "alter ", "create ", } result := input + lowerResult := strings.ToLower(result) for _, d := range dangerous { - result = strings.ReplaceAll(result, d, "") + // Find all occurrences case-insensitively and remove them + for { + idx := strings.Index(lowerResult, d) + if idx == -1 { + break + } + // Remove from both result and lowerResult + result = result[:idx] + result[idx+len(d):] + lowerResult = lowerResult[:idx] + lowerResult[idx+len(d):] + } } return result default: From 8cdc3530294d29c86f04d588f6c26557d0272b89 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 31 Dec 2025 07:22:42 +0000 Subject: [PATCH 4/6] fix: Fixed SQL quoting for empty/zero values and sanitized match filter - Sanitize colval immediately after extraction in match= filter - Fixed empty/zero value handling to use proper SQL literals (0 vs '') - Applied proper quoting for string vs numeric comparisons - Fixed x-fieldfilter handlers for proper value handling Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com> --- pkg/funcspec/function_api.go | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/pkg/funcspec/function_api.go b/pkg/funcspec/function_api.go index b8469c1..f1022d7 100644 --- a/pkg/funcspec/function_api.go +++ b/pkg/funcspec/function_api.go @@ -522,10 +522,16 @@ func (h *Handler) SqlQuery(sqlquery string, options SqlQueryOptions) HTTPFuncTyp if strings.HasPrefix(kLower, "x-fieldfilter-") { colname := strings.ReplaceAll(kLower, "x-fieldfilter-", "") if strings.Contains(strings.ToLower(sqlquery), colname) { - if val == "" || val == "0" { - sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("COALESCE(%s, 0) = %s", ValidSQL(colname, "colname"), ValidSQL(val, "colvalue"))) + if val == "0" { + sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("COALESCE(%s, 0) = 0", ValidSQL(colname, "colname"))) + } else if val == "" { + sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("(%[1]s = '' OR %[1]s IS NULL)", ValidSQL(colname, "colname"))) } else { - sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s = %s", ValidSQL(colname, "colname"), ValidSQL(val, "colvalue"))) + if IsNumeric(val) { + sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s = %s", ValidSQL(colname, "colname"), ValidSQL(val, "colvalue"))) + } else { + sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s = '%s'", ValidSQL(colname, "colname"), ValidSQL(val, "colvalue"))) + } } } } @@ -718,11 +724,17 @@ func (h *Handler) mergeQueryParams(r *http.Request, sqlquery string, variables m } else { if strings.Contains(val, "match=") { colval := strings.ReplaceAll(val, "match=", "") + colval = ValidSQL(colval, "colvalue") // Sanitize immediately if colval != "*" { - sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s ILIKE '%%%s%%'", ValidSQL(parmk, "colname"), ValidSQL(colval, "colvalue"))) + sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s ILIKE '%%%s%%'", ValidSQL(parmk, "colname"), colval)) } } else if val == "" || val == "0" { - sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("(%[1]s = %[2]s OR %[1]s IS NULL)", ValidSQL(parmk, "colname"), ValidSQL(val, "colvalue"))) + // For empty/zero values, treat as literal 0 or empty string with quotes + if val == "0" { + sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("(%[1]s = 0 OR %[1]s IS NULL)", ValidSQL(parmk, "colname"))) + } else { + sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("(%[1]s = '' OR %[1]s IS NULL)", ValidSQL(parmk, "colname"))) + } } else { if IsNumeric(val) { sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s = %s", ValidSQL(parmk, "colname"), ValidSQL(val, "colvalue"))) @@ -763,10 +775,16 @@ func (h *Handler) mergeHeaderParams(r *http.Request, sqlquery string, variables // Handle special headers if strings.Contains(k, "x-fieldfilter-") { colname := strings.ReplaceAll(k, "x-fieldfilter-", "") - if val == "" || val == "0" { - sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("COALESCE(%s, 0) = %s", ValidSQL(colname, "colname"), ValidSQL(val, "colvalue"))) + if val == "0" { + sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("COALESCE(%s, 0) = 0", ValidSQL(colname, "colname"))) + } else if val == "" { + sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("(%[1]s = '' OR %[1]s IS NULL)", ValidSQL(colname, "colname"))) } else { - sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s = %s", ValidSQL(colname, "colname"), ValidSQL(val, "colvalue"))) + if IsNumeric(val) { + sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s = %s", ValidSQL(colname, "colname"), ValidSQL(val, "colvalue"))) + } else { + sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s = '%s'", ValidSQL(colname, "colname"), ValidSQL(val, "colvalue"))) + } } } From a7cc42044b66c99bdb8308c480bad5069900d569 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 31 Dec 2025 07:24:56 +0000 Subject: [PATCH 5/6] fix: Improved SQL injection protection with proper handling - Fixed IN clause to conditionally quote only string values (not numeric) - Fixed LIKE pattern sanitization to preserve wildcards while preventing injection - Improved dangerous pattern removal with case-insensitive regex while preserving case - All funcspec tests now pass (except pre-existing TestReplaceMetaVariables) Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com> --- pkg/funcspec/function_api.go | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/pkg/funcspec/function_api.go b/pkg/funcspec/function_api.go index f1022d7..88f2288 100644 --- a/pkg/funcspec/function_api.go +++ b/pkg/funcspec/function_api.go @@ -713,18 +713,25 @@ func (h *Handler) mergeQueryParams(r *http.Request, sqlquery string, variables m // Apply filters if allowed if allowFilter && len(parmk) > 1 && strings.Contains(strings.ToLower(sqlquery), strings.ToLower(parmk)) { if len(parmv) > 1 { - // Sanitize each value in the IN clause and wrap in quotes + // Sanitize each value in the IN clause with appropriate quoting sanitizedValues := make([]string, len(parmv)) for i, v := range parmv { - sanitized := ValidSQL(v, "colvalue") - // Wrap each value in single quotes for SQL IN clause - sanitizedValues[i] = fmt.Sprintf("'%s'", sanitized) + if IsNumeric(v) { + // Numeric values don't need quotes + sanitizedValues[i] = ValidSQL(v, "colvalue") + } else { + // String values need quotes + sanitized := ValidSQL(v, "colvalue") + sanitizedValues[i] = fmt.Sprintf("'%s'", sanitized) + } } sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s IN (%s)", ValidSQL(parmk, "colname"), strings.Join(sanitizedValues, ","))) } else { if strings.Contains(val, "match=") { colval := strings.ReplaceAll(val, "match=", "") - colval = ValidSQL(colval, "colvalue") // Sanitize immediately + // Don't sanitize LIKE patterns as it would escape wildcards + // Just remove single quotes to prevent SQL injection + colval = strings.ReplaceAll(colval, "'", "''") if colval != "*" { sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s ILIKE '%%%s%%'", ValidSQL(parmk, "colname"), colval)) } @@ -908,18 +915,12 @@ func ValidSQL(input, mode string) string { "exec ", "execute ", "union ", "declare ", "alter ", "create ", } result := input - lowerResult := strings.ToLower(result) + // Use case-insensitive replacement via regex for _, d := range dangerous { - // Find all occurrences case-insensitively and remove them - for { - idx := strings.Index(lowerResult, d) - if idx == -1 { - break - } - // Remove from both result and lowerResult - result = result[:idx] + result[idx+len(d):] - lowerResult = lowerResult[:idx] + lowerResult[idx+len(d):] - } + // Create case-insensitive regex for the pattern + pattern := "(?i)" + regexp.QuoteMeta(d) + re := regexp.MustCompile(pattern) + result = re.ReplaceAllString(result, "") } return result default: From a6a17d019fcd2c3b97039edf83467711ee3dc020 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 31 Dec 2025 07:28:16 +0000 Subject: [PATCH 6/6] fix: Optimized regex performance and added backslash escaping for LIKE patterns - Added backslash escaping to LIKE pattern sanitization - Optimized ValidSQL select mode with single pre-compiled regex - All funcspec tests pass (except pre-existing TestReplaceMetaVariables) Co-authored-by: warkanum <208308+warkanum@users.noreply.github.com> --- pkg/funcspec/function_api.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/pkg/funcspec/function_api.go b/pkg/funcspec/function_api.go index 88f2288..b13ff56 100644 --- a/pkg/funcspec/function_api.go +++ b/pkg/funcspec/function_api.go @@ -729,8 +729,9 @@ func (h *Handler) mergeQueryParams(r *http.Request, sqlquery string, variables m } else { if strings.Contains(val, "match=") { colval := strings.ReplaceAll(val, "match=", "") - // Don't sanitize LIKE patterns as it would escape wildcards - // Just remove single quotes to prevent SQL injection + // Escape single quotes and backslashes for LIKE patterns + // But don't escape wildcards % and _ which are intentional + colval = strings.ReplaceAll(colval, "\\", "\\\\") colval = strings.ReplaceAll(colval, "'", "''") if colval != "*" { sqlquery = sqlQryWhere(sqlquery, fmt.Sprintf("%s ILIKE '%%%s%%'", ValidSQL(parmk, "colname"), colval)) @@ -910,19 +911,14 @@ func ValidSQL(input, mode string) string { // For SELECT clauses, be more permissive but still safe // Remove semicolons and common SQL injection patterns (case-insensitive) dangerous := []string{ - ";", "--", "/*", "*/", "xp_", "sp_", + ";", "--", "/\\*", "\\*/", "xp_", "sp_", "drop ", "delete ", "truncate ", "update ", "insert ", "exec ", "execute ", "union ", "declare ", "alter ", "create ", } - result := input - // Use case-insensitive replacement via regex - for _, d := range dangerous { - // Create case-insensitive regex for the pattern - pattern := "(?i)" + regexp.QuoteMeta(d) - re := regexp.MustCompile(pattern) - result = re.ReplaceAllString(result, "") - } - return result + // Build a single regex pattern with all dangerous keywords + pattern := "(?i)(" + strings.Join(dangerous, "|") + ")" + re := regexp.MustCompile(pattern) + return re.ReplaceAllString(input, "") default: return input }