fix(funcspec): refine filter application logic for SQL queries

* update filter checks to only consider SELECT list
* add test for function parameters not matching filters
This commit is contained in:
Hein
2026-05-15 14:28:12 +02:00
parent ae9e06c98b
commit 2557005f6b
2 changed files with 70 additions and 3 deletions

View File

@@ -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]") {

View File

@@ -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) {