Skip to content
This repository has been archived by the owner on Jan 5, 2023. It is now read-only.

Go: add pgx sqli query #607

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

japroc
Copy link

@japroc japroc commented Nov 15, 2021

Hello, team!

The idea of this PR is to improve default SqlInjection.ql query by adding github.com/jackc/pgx module and related.

I basically reused existing SqlInjection.ql query, and created a custom PgxSqlInjection.ql query. The CodeQL custom module with implements pgx sql argument is defined in Pgx.qll file. I think that pgx support should be implemented by extending SQL::QueryString.

Also i met stange behavior. When i create custom Query class by extending DataFlow::Node the query works fine. But when i extend SQL::QueryString. I do not understand why. Maybe you can support with that bug?

Thanks,
Evgenii.

@japroc japroc requested a review from a team as a code owner November 15, 2021 18:39
result = package(["github.com/jackc/pgx"], "") and result != pgx3PackagePath()
}

string pgxAnyPackagePath() { result = package(["github.com/jackc/pgx"], "") }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
string pgxAnyPackagePath() { result = package(["github.com/jackc/pgx"], "") }
string pgxAnyPackagePath() { result = package("github.com/jackc/pgx", "") }

No need for a set literal with only one element in it. The same applies in many other places.

Copy link
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. All of the modelled sinks appear to be correct. It would be great to have some tests.

I'm not sure exactly what you mean about extending SQL:QueryString not working. An example of how it should be done is BeegoOrm.qll - note that the classes extend SQL::QueryString::Range, rather than SQL::QueryString. I don't think that it's important to change it for this PR though - when we promote this from experimental we can do it in the preferred way.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants