Skip to content

Potential fix for code scanning alert no. 75: SQL query built from user-controlled sources #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
richard-churchman wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Potential fix for code scanning alert no. 75: SQL query built from user-controlled sources #53

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0530585
Potential fix for code scanning alert no. 75: SQL query built from us…
richard-churchman Feb 22, 2025
557d93b
Update dotnet.yml to .Net 9
richard-churchman Feb 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions Jube.Data/Reporting/Postgres.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,17 @@
{
await connection.OpenAsync();

var command = new NpgsqlCommand(sql);
var command = new NpgsqlCommand();
command.Connection = connection;

for (var i = 0; i < parameters.Count; i++)
command.Parameters.AddWithValue("@" + (i + 1), parameters[i]);
{
var paramName = "@param" + (i + 1);
sql = sql.Replace("@" + (i + 1), paramName);
command.Parameters.AddWithValue(paramName, parameters[i]);
}

command.CommandText = sql;

Check failure

Code scanning / CodeQL

SQL query built from user-controlled sources High

This query depends on
this ASP.NET Core MVC action method parameter
Loading
.
Copy link
Contributor Author

@richard-churchman richard-churchman Feb 22, 2025

Choose a reason for hiding this comment

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

This automated suggestion is way off.

I will use this branch however to remove the direct path for string concatenation. I believe in the commit that was intended to address this, there is already some checking that the values passed are valid based on model configuration. Here I will go a step further and look the values up, and concatenate them the value returned from various dictionaries or lists (they should after all be the same value). In the absence of the values available, will error it out.

await command.PrepareAsync();
}
catch
Expand Down
Loading