Go : Add query to detect DSN injections

github / securitylab

Resources related to GitHub Security Lab

https://securitylab.github.com

MIT License

1.4k stars 247 forks source link

Go : Add query to detect DSN injections #748

Closed porcupineyhairs closed 1 year ago

porcupineyhairs commented 1 year ago

Query PR

https://github.com/github/codeql/pull/12901

Language

GoLang

CVE(s) ID list

CVE-2022-3023

CWE

CWE-134

Report

Data-Source Name injection vulnerability occurs when a untrusted input is used to generate the SQL connection string. This query detects DSN Injection by using taint-flow analysis. To avoid FP's, the query filters any regex check on the taint as a sanitizer. This successfully detects CVE-2022-3023 occuring in pingcap/tidb(33.9k stars, 5.5k forks). I have attached the database to verify the vulnerability below.

DB link (valid for 7 days) : https://ufile.io/pbdyde5m

PS: This issue is being created in light of this discussion.

Are you planning to discuss this vulnerability submission publicly? (Blog Post, social networks, etc).

[ ] Yes
[ ] No

Blog post link

No response

porcupineyhairs commented 1 year ago

@xcorail This issue has not received initial triage since it was filed last week. Can you please confirm that you have downloaded the db I share on the link? The link would expire tomorrow.

JarLob commented 1 year ago

Thank you for the heads up, the db was backed up.

porcupineyhairs commented 1 year ago

@xcorail @JarLob Any updates here?

ghsecuritylab commented 1 year ago

Your submission is now in status Test run.

For information, the evaluation workflow is the following: Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

ghsecuritylab commented 1 year ago

Your submission is now in status Results analysis.

For information, the evaluation workflow is the following: Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

JarLob commented 1 year ago

Hi @porcupineyhairs, May I suggest these changes:

The file read is specific to MySQL and this is the driver name you are using in tests. However the query doesn't check it (Driver name - the first argument of db.Open). While any tainted data source name string (second argument of db.Open) may have unwanted effect on the program, I feel that either it has to be made MySQL specific, or additional two MySQL specific queries (local and remote) are needed.
While the CVE is marked as CWE-134, IMHO this is a mistake because the formatting string is not tainted, nor your query is looking for formatting. I think CWE-74 (Improper neutralization of special elements... Injection) would be the right one.
This query depends on a user-provided value. is cryptic IMHO. Data-Source Name is built using untrusted user input. from the help file sounds better.

porcupineyhairs commented 1 year ago

@JarLob I have dropped in a new PR https://github.com/github/codeql/pull/13644 with the changes you suggest. PTAL

porcupineyhairs commented 1 year ago

@xcorail This ticket was without updates for a sometime before I posted a response earlier this week. The Github tracker does not show any assignees for reviewing this ticket. Can you check and confirm that there are people assigned for reviewing #748 and #757 ?

ghsecuritylab commented 1 year ago

Your submission is now in status Query review.

For information, the evaluation workflow is the following: Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

ghsecuritylab commented 1 year ago

Your submission is now in status Final decision.

For information, the evaluation workflow is the following: Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

porcupineyhairs commented 1 year ago

@ghsecuritylab This got to Final decision without completing query review.

ghsecuritylab commented 1 year ago

Your submission is now in status Pay.

For information, the evaluation workflow is the following: Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed

xcorail commented 1 year ago

Created Hackerone report 2076517 for bounty 498282 : [748] Go : Add query to detect DSN injections

ghsecuritylab commented 1 year ago

Your submission is now in status Closed.

For information, the evaluation workflow is the following: Initial triage > Test run > Results analysis > Query review > Final decision > Pay > Closed