diff --git a/pkg/funcspec/function_api.go b/pkg/funcspec/function_api.go index a5231a9..ff5eb2d 100644 --- a/pkg/funcspec/function_api.go +++ b/pkg/funcspec/function_api.go @@ -729,9 +729,10 @@ func (h *Handler) mergeQueryParams(r *http.Request, sqlquery string, variables m propQry[parmk] = val } - // Apply filters if allowed — check against string-literal-stripped SQL to avoid - // matching column names that only appear inside quoted arguments (e.g. JSON strings) - if allowFilter && len(parmk) > 1 && strings.Contains(strings.ToLower(sqlStripStringLiterals(sqlquery)), strings.ToLower(parmk)) { + // Apply filters if allowed — check only the SELECT list to avoid matching function + // parameters in the FROM clause (e.g. [p_rid_doctype] in a set-returning function call) + // or names inside quoted string arguments. + if allowFilter && len(parmk) > 1 && strings.Contains(sqlSelectList(sqlStripStringLiterals(sqlquery)), strings.ToLower(parmk)) { if len(parmv) > 1 { // Sanitize each value in the IN clause with appropriate quoting sanitizedValues := make([]string, len(parmv)) @@ -847,6 +848,18 @@ func sqlStripStringLiterals(sql string) string { return re.ReplaceAllString(sql, "''") } +// sqlSelectList returns the column list portion of a SELECT query (between SELECT and FROM). +// Returns the full query lowercased if no clear SELECT…FROM boundary is found. +func sqlSelectList(sql string) string { + lower := strings.ToLower(sql) + selectPos := strings.Index(lower, "select ") + fromPos := strings.Index(lower, " from ") + if selectPos < 0 || fromPos <= selectPos { + return lower + } + return lower[selectPos+7 : fromPos] +} + // replaceMetaVariables replaces meta variables like [rid_user], [user], etc. in the SQL query func (h *Handler) replaceMetaVariables(sqlquery string, r *http.Request, userCtx *security.UserContext, metainfo map[string]interface{}, variables map[string]interface{}) string { if strings.Contains(sqlquery, "[p_meta_default]") { diff --git a/pkg/funcspec/function_api_test.go b/pkg/funcspec/function_api_test.go index 505a25b..39f6076 100644 --- a/pkg/funcspec/function_api_test.go +++ b/pkg/funcspec/function_api_test.go @@ -957,6 +957,60 @@ func TestAllowFilterDoesNotMatchInsideJsonArgument(t *testing.T) { } } +// TestAllowFilterDoesNotMatchFunctionParams verifies that query params that appear only +// as function call arguments in the FROM clause (e.g. [p_rid_doctype]) are not treated +// as column filters, since they are not in the SELECT list. +func TestAllowFilterDoesNotMatchFunctionParams(t *testing.T) { + handler := NewHandler(&MockDatabase{}) + + sqlQuery := `select rid, rid_parent, description, row_cnt, filterstring, tableprefix, rid_table, tooltip, additionalfilter, haschildren + from crm_get_doc_menu($JQ$[p_tableprefix]$JQ$,[p_rid_parent],[p_rid_doctype],[p_removedup],[p_showall]) r` + + tests := []struct { + name string + queryParams map[string]string + checkResult func(t *testing.T, result string) + }{ + { + name: "p_rid_doctype is a function param, not a column — no filter applied", + queryParams: map[string]string{"p_rid_doctype": "0"}, + checkResult: func(t *testing.T, result string) { + if strings.Contains(strings.ToLower(result), "where") { + t.Errorf("Expected no WHERE clause for p_rid_doctype (function arg, not SELECT column), got:\n%s", result) + } + }, + }, + { + name: "p_showall is a function param, not a column — no filter applied", + queryParams: map[string]string{"p_showall": "1"}, + checkResult: func(t *testing.T, result string) { + if strings.Contains(strings.ToLower(result), "where") { + t.Errorf("Expected no WHERE clause for p_showall (function arg, not SELECT column), got:\n%s", result) + } + }, + }, + { + name: "rid is a SELECT column — filter applied", + queryParams: map[string]string{"rid": "42"}, + checkResult: func(t *testing.T, result string) { + if !strings.Contains(strings.ToLower(result), "where") { + t.Error("Expected WHERE clause for rid (real SELECT column)") + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := createTestRequest("GET", "/test", tt.queryParams, nil, nil) + variables := make(map[string]interface{}) + propQry := make(map[string]string) + result := handler.mergeQueryParams(req, sqlQuery, variables, true, propQry) + tt.checkResult(t, result) + }) + } +} + // TestGetReplacementForBlankParamDoubleQuote verifies that placeholders surrounded by // double quotes (as in JSON string values) are blanked to "" not NULL. func TestGetReplacementForBlankParamDoubleQuote(t *testing.T) {