search

github / securitylab

Resources related to GitHub Security Lab
https://securitylab.github.com
MIT License
1.41k stars 244 forks source link

Go: Add modelling for `gorqlite` and `GoFrame` frameworks #723

Closed porcupineyhairs closed 1 year ago

porcupineyhairs commented 1 year ago

Query PR

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

Language

GoLang

CVE(s) ID list

alexedwards/scs

CWE

CWE-89: SQL Injection

Report

The PR improves the existing modelling for Go's SQL injection query. It adds modelling for gogf(8.7k stars) and gorqlite(87 stars) frameworks.

There is atleast one CVE detected by this query. There should be more detected but given ongoing issue with rate limits on MRVA, I am unable to test my query at scale.

The original project needs some work to be correctly build and detect the vulnerability. I am attaching a database with a trimmed down version of the vulnerable project to show successful detection.

scsDb.zip

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

Blog post link

No response

porcupineyhairs commented 1 year ago

@ghsecuritylab Any updates here?

JarLob commented 1 year ago

Hi @porcupineyhairs, We are working on this. Please note though, that a technical requirement for successful submission is CVE number, but this vulnerability has no CVE assigned for some reason. We have reached out to maintainer asking if they willing to request the CVE.

porcupineyhairs commented 1 year ago

@JarLob I tried reaching out to the maintainer too. They haven't responded. Did you receive any response? Can this issue be processed without a CVE? There is already a coordinated vulnerability disclosure made and a report published. The only thing left is a CVE.

JarLob commented 1 year ago

Hi @porcupineyhairs, I will ask internally what is the current status and someone will get back to you shortly.

p- commented 1 year ago

Hello @porcupineyhairs We were not able to verify with the maintainer of scs if a CVE should be assigned to this vulnerability. Basically, there are now two options: you could ask huntr.dev if they can assign a CVE for this issue (huntr.dev is a CNA). Alternatively, you could ask Mitre if they would assign a CVE for it. (The last option of course would be to search for another vulnerability using this query where a CVE can be assigned.)

porcupineyhairs commented 1 year ago

@p- Huntr does not issue CVE when a third party asks for it. They only issue a CVE if the maintaienr asks for it.

I have asked MITRE to issue a CVE. Let's see what they say. Looks like this issue is going to be pending for a while.

porcupineyhairs commented 1 year ago

@p- I spoke with MITRE. They have declined issuing a CVE for this particular patch.

Here is there response.


As far as we can tell, SQL injection was only in rqlite, which
was experimental code that was never released in any SCS version.
The code was added in:

  https://github.com/alexedwards/scs/commit/b95d685652e43a5e3662740fa627fec2f21079ab

and then soon deleted in:

  https://github.com/alexedwards/scs/commit/d93ace5be94bc476d79a2b818ae6579fa76e5a59

Because it was never released, there is no CVE ID.

I have disputed this. In the meantime, I have sent in a new PR github/codeql#12901 . This is a smaller scoped query which may not have met the bounty programs norms but with the rest of the improvements, it should ideally meet the medium severity norms. It detects CVE-2022-3023. I am trying to attach the DB here but it says it's too big(534 MB). So, it seems like you would have to generate it yourself.

Let me know if there's anything you need to move this issue forward.

p- commented 1 year ago

Hi @porcupineyhairs I will discuss this with the team. From my current/initial point of view it might make more sense if the PR for DSN Injection would be submitted as a new bounty submission? (But yes if you're worried about the scope we might have a look at it both ways).

As to the answer from MITRE:

As far as we can tell, SQL injection was only in rqlite, which
was experimental code that was never released in any SCS version.
The code was added in:

  https://github.com/alexedwards/scs/commit/b95d685652e43a5e3662740fa627fec2f21079ab

and then soon deleted in:

  https://github.com/alexedwards/scs/commit/d93ace5be94bc476d79a2b818ae6579fa76e5a59

Because it was never released, there is no CVE ID.

This sounds reasonable to me and resembles the experience of finding no further TPs with these additions. Of course this doesn't mean that your initial additions are worthless, it just might mean that your initial additions are not eligible for a bounty payout.

porcupineyhairs commented 1 year ago

@p- The PR is in an advanced stage of review by the QL team. Moving to a new issue may be counter-intuitive. I have however, created a new issue for this as you ask. I have also included a link to a QL db there with the vulnerable code.

I am closing this issue now.

ghsecuritylab commented 5 months 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