github / securitylab

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

[Ruby] Add Unicode Bypass Validation query, test and help file #750

Closed Sim4n6 closed 2 months ago

Sim4n6 commented 1 year ago

Query PR

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

Language

Ruby

CVE(s) ID list

CWE

CWE-176

Report

Please refer to the report in this issue: https://github.com/github/securitylab/issues/749

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

Blog post link

https://sim4n6.beehiiv.com

jorgectf commented 1 year ago

👋 @Sim4n6

Thank you for your contribution!

The CVE linked does not relate to the contribution itself, as the code in question is not Ruby. Could you provide a CVE that matches the contribution fully?

Sim4n6 commented 1 year ago

Well, indeed, CVE is related to the Python codebase. But the GoSecure research mentions the same behavior as in https://gosecure.github.io/unicode-pentester-cheatsheet/

Sim4n6 commented 1 year ago

Hi @jorgectf ,

I will be hunting at scale for a CVE that matches the contribution codebase.

I almost found one (did fail at the end) for JavaScript too, check this commit https://github.com/isaacs/node-tar/commit/4501bdbe59fb56dbc0de6e7e220340aaaef9394d.

This comment is valid for the Ruby , GoLang, Java and CSharp contributions and the coming query for Javascript.

I believe this would take a bit of time. Meanwhile, I would perfect the query using the reviewers' reviews.

Have a nice weekend. @Sim4n6

jorgectf commented 1 year ago

👋 @Sim4n6

That sounds great! Feel free to ping me as soon as you have a CVE for any of the submissions you mentioned.

Sim4n6 commented 1 year ago

Hi @jorgectf ,

I identified a case in RubyGems. The advisory is published at https://github.com/shirasagi/shirasagi/security/advisories/GHSA-xr45-c2jv-2v9r. The CVE-2023-41889 is being requested...

I hope this one is done for sure.

Regards @Sim4n6

Sim4n6 commented 1 year ago

And it is published as a medium severity vulnerability https://nvd.nist.gov/vuln/detail/CVE-2023-41889

Have a nice weekend @Sim4n6

Sim4n6 commented 1 year ago

Hi @jorgectf , The CVE-2023-41889 is published. Please keep me posted about the result of this one. Thank you

Sim4n6 commented 1 year ago

By the way @jorgectf , I don't know much details about the next step, you could explain more? I may provide further details. I mean do you need me take some screenshots the vulnerable commit for instance or something ...

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

jorgectf commented 1 year ago

@Sim4n6 given that the submission is a full new query, can you provide us with the database that finds the CVE you have linked? I will continue with the process in the meantime.

Sim4n6 commented 1 year ago

@jorgectf I believe it is this commit https://github.com/shirasagi/shirasagi/tree/f249ce3f06f6bfbc0017b38f5c13de424334c3ea. I have set up a fresh db https://wormhole.app/oNDjb#imWwGg_fhkXJzGXpcnQDlg (expiring link). The advisory showcase the sink & the method from the additional taint step https://github.com/shirasagi/shirasagi/security/advisories/GHSA-xr45-c2jv-2v9r

jorgectf commented 11 months ago

Hi @Sim4n6,

I can't find a hit in the database for https://github.com/shirasagi/shirasagi (f249ce3f06f6bfbc0017b38f5c13de424334c3ea) using your query.

Sim4n6 commented 11 months ago

Hi @jorgectf, I'll check during this weekend and get back to you as soon as possible.

Sim4n6 commented 11 months ago

Hi @jorgectf ,

After further investigation, this is how I identified the vulnerability reported GHSA-xr45-c2jv-2v9r:

  1. The sink was caught:

    sink
  2. The Unicode normalization input contains a "Call to strip". The method strip is considered within the additional TaintStep of the query strip. Which could be bypassed using some specific Unicode characters.

  3. From there, I went manual because somehow the flow was lost due to the absence of an AdditionalJumpStep that would consider the callback before_validation: normalize_name.

What do you think, about how we should proceed, please?

jorgectf commented 11 months ago

Hi @Sim4n6, thank you for the detailed analysis! I would suggest you model everything needed for the query to catch the flow linked to the CVE.

jorgectf commented 4 months ago

👋 Hello @Sim4n6, what is the status of your progress? Were you able to work on this?

Sim4n6 commented 4 months ago

I did not work on it, but I'm learning ruby from the start... I have it on my mind, I would spend the weekend on it.

Sim4n6 commented 4 months ago

Hey @jorgectf ,

The query Unicode Bypass Validation encompasses only issues where the Source is user-untrusted data. But when I've been working on Shirasagi repository, I've been thinking more about Unicode use mishandling by calling some methods just before a Unicode transformation which may lead to resurfacing some characters (more precisely with Compatibility forms like NFKD or NFKC).

The additional steps where the state is being switched could be considered a Source of Unicode mishandling method calls, for instance:

  1. Source: Call to Strip() on a name.
  2. Sink: Unicode Compatibility transformation.

With such a behavior, I could have a name fully empty with some specific Unicode characters. For instance, it's a functional and security issue that could lead to DoS.

So I made the PR https://github.com/github/codeql/pull/16471 accordingly, which detects this one in the vulnerable version or the repo Shirasagi (and accidentally another but a fixed one).

image

Sim4n6 commented 4 months ago

The fix commit is https://github.com/shirasagi/shirasagi/pull/5020/commits/ca469fdbd87659049996833effe3c9f682772a5d

Sim4n6 commented 4 months ago

CVE ID should be for this one CVE-2023-41889.

Sim4n6 commented 4 months ago

hey @jorgectf , for this one, if you need me to provide any further details please let me know.

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

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

Sim4n6 commented 3 months ago

hey @jorgectf , this one is ready it won't be affected by the sunsetting of the BBP, right ?

jorgectf commented 3 months ago

No, this submission won't be affected @Sim4n6

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

ghsecuritylab commented 2 months 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 2 months ago

Created Hackerone report 2573453 for bounty 591009 : [750] [Ruby] Add Unicode Bypass Validation query, test and help file

ghsecuritylab commented 2 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