Where to validate input parameters to prevent SQL injection in C# web API controller or service or repository?

Question

I understand that parameterized queries is sufficient for preventing SQL injection. Is it good practice to validate the input parameters as well? What's a good way to validate the input parameters and where (controller, service, repository) do I validate them?

This is my controller method:

public async Task<ActionResult<List<Level>>> GetLevelsAsync([FromRoute] string Code, string Year)
{
    int appId = IsoCode.FromName(Code).Id;
    var result = await _sampleService.GetLevelsAsync(appId, Year);
    return result;
}

This is my service method:

public async Task<List<Level>> GetLevelsAsync(int appId, string year)
{
    if (string.IsNullOrWhiteSpace(year) == true)
        throw new ArgumentNullException(nameof(year));

    var result = await _sampleRepository.GetLevelsByYear(appId, year);
    return result;
}

This is my repository method:

public async Task<List<Level>> GetLevelsByYear(int appId, string year)
{
    if (string.IsNullOrWhiteSpace(year))
        throw new ArgumentNullException(nameof(year));

    var result = new List<Level>();
    var parameters = new { AppId = appId, Year = year };
    string sql = @"
                SELECT * 
                FROM [Levels] as l, [LevelYears] as v 
                WHERE 
                    v.LevelId = l.Id AND l.Active = 1 AND l.AppId = @AppId AND v.Year = @Year
                ORDER BY v.Sort asc
        ";

    using (IDbConnection db = new SqlConnection(_settings.SqlServerConnString))
    {
        try
        {
            result = db.Query<Level>(sql, parameters).ToList();
        }
        catch (Exception e)
        {
            _logger.LogError(e, "Error querying levels by year", new { appId, year });
            throw;
        }
    }
    return result;
}

Personally, I'd say that validating input before failing at the SQL level is probably a good practice. Look into the `FluentValidation` NuGet packages. — Daevin, Sep 08 '22 at 15:14
You don't need to validate anything, it's unnecessary. Parameters are not injectable unless badly compiled into dynamic SQL — Charlieface, Sep 08 '22 at 16:58

score 4 · Answer 1 · answered Sep 09 '22 at 16:52

Validating is good defensive coding. Some developers (not you, I'm sure) treat data in the database as "trusted data" and don't properly leverage encoding or prepared statements, which can lead to follow on issues such as second order SQL injection, stored XSS, or even business logic issues.

Validation is context dependent. Positive validation (white list validation) is a best practice, but it's not always possible. Some examples:

You can often check that a particular value is one of a set of known values (white list validation)
If a field is supposed to be an integer or date, you can validate this field to ensure it contains that data type (or maybe NULL).
You can require most fields to have a minimum and maximum length.
You should verify that any string contains only valid characters for its encoding (e.g., no invalid UTF-8 sequences and only printable characters) - this can often be done more generally in a WAF or servlet filter

Where to validate? You should always validate when data passes a trust boundary - validate any data coming from another source. The validation should be at the layers that are likely to be reused. In this case, I'd recommend validating at the service level in this case.

Where to validate input parameters to prevent SQL injection in C# web API controller or service or repository?

1 Answers1