Skip to content

fix: Comprehensive SQL injection protection in funcspec handlers#17

Merged
warkanum merged 6 commits intofix-funcapifrom
copilot/sub-pr-16
Dec 31, 2025
Merged

fix: Comprehensive SQL injection protection in funcspec handlers#17
warkanum merged 6 commits intofix-funcapifrom
copilot/sub-pr-16

Conversation

Copy link
Contributor

Copilot AI commented Dec 31, 2025

The original security fix added ValidSQL sanitization but missed multiple SQL injection vectors where user-controlled input was directly concatenated into queries.

Vulnerabilities Fixed

Direct string replacement injection:

  • Path parameters in mergePathParams - no sanitization before strings.ReplaceAll
  • Query parameters with p- prefix in mergeQueryParams
  • Header parameters in mergeHeaderParams

IN clause injection:

// Before: unsanitized array concatenation
sqlquery = fmt.Sprintf("%s IN (%s)", ValidSQL(colname, "colname"), strings.Join(parmv, ","))

// After: sanitize each value, conditionally quote strings vs numerics
sanitizedValues := make([]string, len(parmv))
for i, v := range parmv {
    if IsNumeric(v) {
        sanitizedValues[i] = ValidSQL(v, "colvalue")
    } else {
        sanitizedValues[i] = fmt.Sprintf("'%s'", ValidSQL(v, "colvalue"))
    }
}

ValidSQL Enhancements

  • Added backslash escaping in colvalue mode (was only escaping single quotes)
  • LIKE pattern handling: escape quotes/backslashes while preserving % and _ wildcards
  • Optimized select mode: single pre-compiled case-insensitive regex instead of per-pattern compilation
  • Extended dangerous keywords: EXEC, EXECUTE, UNION, DECLARE, ALTER, CREATE

Known Limitation

Placeholder replacements ([param]) require proper quoting in SQL templates. Example:

-- Vulnerable: WHERE name = [username]
-- Safe: WHERE name = '[username]'

Fixing this would break existing templates - should be addressed separately.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 5 commits December 31, 2025 07:19
- 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>
- 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>
- 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>
- 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>
…E 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>
Copilot AI changed the title [WIP] Fix possible injections in funcapi fix: Comprehensive SQL injection protection in funcspec handlers Dec 31, 2025
Copilot AI requested a review from warkanum December 31, 2025 07:29
@warkanum warkanum marked this pull request as ready for review December 31, 2025 07:35
@warkanum warkanum merged commit 191822b into fix-funcapi Dec 31, 2025
@warkanum warkanum deleted the copilot/sub-pr-16 branch December 31, 2025 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants