Skip to content

EF1003: Detect explicit string.Format/string.Concat in raw SQL APIs#38208

Merged
AndriySvyryd merged 2 commits into
dotnet:mainfrom
m-x-shokhzod:fix/analyzer-detect-string-format-concat
Jun 8, 2026
Merged

EF1003: Detect explicit string.Format/string.Concat in raw SQL APIs#38208
AndriySvyryd merged 2 commits into
dotnet:mainfrom
m-x-shokhzod:fix/analyzer-detect-string-format-concat

Conversation

@m-x-shokhzod
Copy link
Copy Markdown
Contributor

Fixes #37915.

Problem

EF1003 (StringConcatenationUsageInRawQueries) currently warns when a raw SQL API receives a string built with the + operator, and EF1002 covers interpolated strings. Equivalent dynamic-string patterns built with explicit method calls were silently allowed:

db.Database.ExecuteSqlRaw(string.Format("UPDATE T SET X = {0}", userInput));
db.Users.FromSqlRaw(string.Concat("SELECT * FROM T WHERE Id = ", userInput));

These have the same SQL-injection profile as the + form but produced no diagnostic.

Fix

Extend StringsUsageInRawQueriesDiagnosticAnalyzer.AnalyzeInvocation with a third pattern: when the SQL argument is itself an IInvocationOperation of string.Format or string.Concat, walk its arguments and report EF1003 if at least one of them is non-constant.

Per the issue title (EF1003: Also detect ...), both string.Format and string.Concat map to EF1003. Happy to split string.Format onto EF1002 if reviewers prefer the semantic split.

Implementation notes:

  • Implicit conversions (notably the stringobject boxing in string.Format(string, object) overloads) are unwrapped via a small helper so a fully constant call like string.Format("X = {0}", "1") is correctly recognized as constant and does not warn.
  • params object?[] arrays are inspected element-by-element rather than tested as a whole.
  • No new diagnostic IDs, no new resource strings, no new release notes — this is a pure rule extension on EF1003.

Verification

  • EFCore.Analyzers builds clean (netstandard2.0).
  • test/EFCore.Analyzers.Tests — full suite passes locally: 114/114 (was 90; +24 new theory cases across the four target methods × six new scenarios).

New test scenarios cover:

  • string.Format(constFormat, constArg) — does not report
  • string.Format(constFormat, parameter) — reports EF1003
  • string.Format(constFormat, GetId()) — reports EF1003
  • string.Concat(const, const, const) — does not report
  • string.Concat(const, parameter) — reports EF1003
  • string.Concat(const, GetId()) — reports EF1003

Each scenario is parameterized over FromSqlRaw, ExecuteSqlRaw, ExecuteSqlRawAsync, and SqlQueryRaw<T>.

The EF1003 analyzer warns when a raw SQL API receives a dynamically
built string. It already covered interpolated strings (EF1002) and the
binary `+` concatenation operator (EF1003), but explicit calls such as

    db.Database.ExecuteSqlRaw(string.Format("UPDATE T SET X={0}", id));
    db.Users.FromSqlRaw(string.Concat("SELECT * FROM T WHERE Id=", id));

were silently allowed. Extend the analyzer to also report these when at
least one argument is non-constant. Implicit conversions (object
boxing in the `string.Format(string, object)` overloads) are unwrapped
so that fully constant calls do not trigger the warning.

Fixes dotnet#37915
@m-x-shokhzod m-x-shokhzod requested a review from a team as a code owner May 1, 2026 05:56
@m-x-shokhzod m-x-shokhzod reopened this May 2, 2026
@m-x-shokhzod
Copy link
Copy Markdown
Contributor Author

@dotnet-policy-service agree

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the EF Core analyzers’ EF1003 rule (StringConcatenationUsageInRawQueries) to also flag raw SQL strings constructed via explicit string.Format(...) and string.Concat(...) calls (in addition to + concatenation and interpolated strings), closing a gap that could allow SQL-injection-prone dynamic SQL to go undetected.

Changes:

  • Extend StringsUsageInRawQueriesDiagnosticAnalyzer.AnalyzeInvocation to recognize string.Format/string.Concat invocations and warn when any relevant argument is non-constant.
  • Add new analyzer tests covering constant vs. non-constant string.Format/string.Concat used in FromSqlRaw, ExecuteSqlRaw, ExecuteSqlRawAsync, and SqlQueryRaw<T>.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/EFCore.Analyzers/StringsUsageInRawQueriesDiagnosticAnalyzer.cs Adds detection for raw SQL built via string.Format / string.Concat based on const-ness of arguments.
test/EFCore.Analyzers.Tests/StringConcatenationInRawQueriesAnalyzerTests.cs Adds new theory cases validating EF1003 behavior for Format/Concat construction patterns.

Comment thread src/EFCore.Analyzers/StringsUsageInRawQueriesDiagnosticAnalyzer.cs
The IFormatProvider argument of the string.Format(IFormatProvider, ...)
overloads (e.g. CultureInfo.InvariantCulture) is never a compile-time
constant, so HasNonConstantArgument reported EF1003 even when the format
string and every formatted value were constant.

Skip that argument by its parameter type — it never contributes to the
SQL text — and evaluate only the format string and formatted values.
string.Concat is unaffected (it has no IFormatProvider overload).

Add a no-diagnostic test for the provider overload and a companion
should-report test to confirm the skip doesn't suppress genuine warnings.
@AndriySvyryd AndriySvyryd merged commit 04b37f3 into dotnet:main Jun 8, 2026
13 checks passed
@AndriySvyryd
Copy link
Copy Markdown
Member

Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EF1003: Also detect explicit calls to string.Format(...) and string.Concat(...)

3 participants