Skip to content

Fix SQL injection in GetGodByNameAsync#8

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/fix-sql-injection-god-query
Draft

Fix SQL injection in GetGodByNameAsync#8
Copilot wants to merge 2 commits intomainfrom
copilot/fix-sql-injection-god-query

Conversation

Copy link

Copilot AI commented Feb 19, 2026

Problem

GetGodByNameAsync used FromSqlRaw() with string interpolation on unsanitized URL parameters, allowing arbitrary SQL injection (CVSS 9.8).

// Vulnerable: user input directly interpolated into SQL
var query = $"SELECT * FROM God WHERE Name LIKE '%{parameter.Name}%'";
var result = _context.Gods.FromSqlRaw(query).ToList();

Changes

  • Replaced raw SQL with parameterized LINQ using EF.Functions.Like() for automatic query parameterization
  • Preserved alias search functionality using .Include() and .Any() for related entity filtering
// Secure: EF Core automatically parameterizes user input
var normalizedName = $"%{parameter.Name}%";
return await _context.Gods
    .Include(g => g.Aliases)
    .Where(g => EF.Functions.Like(g.Name, normalizedName) 
             || g.Aliases.Any(a => EF.Functions.Like(a.Name, normalizedName)))
    .ToListAsync();

Testing

Added unit and integration tests verifying common injection payloads (' OR '1'='1, '; DROP TABLE) return empty results instead of bypassing filters or executing commands.

Original prompt

This section details on the original issue you should resolve

<issue_title>🔴 CRITICAL: SQL Injection Vulnerability in GetGodByNameAsync</issue_title>
<issue_description>## Description

The GetGodByNameAsync method in GodRepository.cs (line 68) constructs SQL queries using raw string interpolation with unsanitized user input, creating a critical SQL injection vulnerability.

Vulnerable Code:

var query = parameter.IncludeAliases 
    ? $"SELECT * FROM God WHERE Name LIKE '%{parameter.Name}%' or Id in (SELECT GodId FROM Alias WHERE Name LIKE '%{parameter.Name}%')" 
    : $"SELECT * FROM God WHERE Name LIKE '%{parameter.Name}%'";
var result = _context.Gods.FromSqlRaw(query).ToList();

The parameter.Name value comes directly from the URL path parameter without any sanitization, allowing attackers to inject arbitrary SQL commands.

Security Implications

  • Data Breach: Attackers can extract sensitive data from any table using UNION attacks
  • Data Destruction: Commands like '; DROP TABLE God; -- could delete entire tables
  • Privilege Escalation: In multi-tenant scenarios, attackers could access other users' data
  • Authentication Bypass: SQLite supports ATTACH DATABASE, potentially allowing access to other database files
  • CVSS Score: 9.8 (Critical) - easily exploitable, no authentication required
  • OWASP 2025 Category: A05:2025 - Injection

Example Attack

# Returns all gods, bypassing the name filter
GET /api/v1/gods/search/' OR '1'='1

# More dangerous payloads:
GET /api/v1/gods/search/'; DELETE FROM God WHERE '1'='1'; --
GET /api/v1/gods/search/' UNION SELECT Id, 'hacker@evil.com', Description, MythologyId FROM God; --

Solution

Replace FromSqlRaw() with LINQ queries that use automatic parameterization.

Recommended Implementation (Pure LINQ)

Replace the entire method in src/Gods/DBRepositories/GodRepository.cs (lines 66-71):

public async Task<List<God>> GetGodByNameAsync(GodByNameParameter parameter)
{
    var normalizedName = $"%{parameter.Name}%";
    
    if (parameter.IncludeAliases)
    {
        var gods = await _context.Gods
            .Include(g => g.Aliases)
            .Where(g => EF.Functions.Like(g.Name, normalizedName) 
                     || g.Aliases.Any(a => EF.Functions.Like(a.Name, normalizedName)))
            .ToListAsync();
        return gods;
    }
    else
    {
        return await _context.Gods
            .Where(g => EF.Functions.Like(g.Name, normalizedName))
            .ToListAsync();
    }
}

Alternative: Parameterized Raw SQL

If LINQ performance is insufficient:

public async Task<List<God>> GetGodByNameAsync(GodByNameParameter parameter)
{
    var searchPattern = $"%{parameter.Name}%";
    
    if (parameter.IncludeAliases)
    {
        return await _context.Gods
            .FromSqlInterpolated($@"
                SELECT DISTINCT g.* 
                FROM God g
                LEFT JOIN Alias a ON g.Id = a.GodId
                WHERE g.Name LIKE {searchPattern} 
                   OR a.Name LIKE {searchPattern}")
            .ToListAsync();
    }
    else
    {
        return await _context.Gods
            .FromSqlInterpolated($"SELECT * FROM God WHERE Name LIKE {searchPattern}")
            .ToListAsync();
    }
}

Verification

Add this unit test to tests/UnitTests/GodEndpointsTests.cs:

[Test]
public async Task GetGodByName_DoesNotAllowSqlInjection()
{
    // Arrange
    var maliciousName = "' OR '1'='1";
    var parameter = new GodByNameParameter(maliciousName, false);
    
    // Act
    var result = await _repository.GetGodByNameAsync(parameter);
    
    // Assert
    Assert.That(result, Is.Empty); // Should find nothing, not all records
}

Files Affected

  • src/Gods/DBRepositories/GodRepository.cs (line 68)
  • src/Endpoints/v1/Gods.cs (line 17 - calls the vulnerable method)

Priority

IMMEDIATE - This is a production-level vulnerability that should be addressed before any further deployments.

References

<agent_instructions>Follow the recommended LINQ-based solution provided in the issue description. Replace the vulnerable FromSqlRaw() implementation with the parameterized LINQ query using EF.Functions.Like(). Ensure the fix handles both the includeAliases=true and includeAliases=false cases correctly. Add the suggested unit test to verify SQL injection protection.</agent_instructions>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Replace FromSqlRaw with EF.Functions.Like for parameterized queries
- Add unit test to verify SQL injection protection
- Add integration test with real SQL injection payloads
- All tests passing (3 unit tests + 4 integration tests)

Co-authored-by: aatmmr <15815012+aatmmr@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix SQL injection vulnerability in GetGodByNameAsync Fix SQL injection in GetGodByNameAsync Feb 19, 2026
Copilot AI requested a review from aatmmr February 19, 2026 13:59
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.

🔴 CRITICAL: SQL Injection Vulnerability in GetGodByNameAsync

2 participants

Comments