Skip to content
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.

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

Catastrophic backtracking on SQL/SQL+Jinja Lexer #2355

Closed
SCH227 opened this issue Feb 26, 2023 · 6 comments
Closed

Catastrophic backtracking on SQL/SQL+Jinja Lexer #2355

SCH227 opened this issue Feb 26, 2023 · 6 comments
Labels
T-bug type: a bug

Comments

@SCH227
Copy link

SCH227 commented Feb 26, 2023

The following pattern is inefficient, it may lead to a Regular expression Denial of Service vulnerability:
r'\{%-?\s*macro \w+\(.*\)\s*-?%\}\s+.*\s+\{%-?\s*endmacro\s*-?%\}'
https://github.com/pygments/pygments/blob/master/pygments/lexers/templates.py#L2295

This was originally reported privately on the security channel, but was asked to open an issue here

@jeanas
Copy link
Contributor

jeanas commented Feb 26, 2023

I'm not sure what you mean by “security channel”, but if you rely on Pygments terminating in reasonable time, don't. Please read https://pygments.org/docs/security/

That said, this indeed look like a bug.

@jeanas jeanas added the T-bug type: a bug label Feb 26, 2023
@SCH227
Copy link
Author

SCH227 commented Feb 26, 2023

By "Security Channel", I meant I reported this privately to Red Hat (which I was informed is the CNA for this project) and also to one of the maintainers (georg@python.org). I preferred to not include the PoC here in public.

I have a new similar one to report. Should I open another issue here?

@jeanas
Copy link
Contributor

jeanas commented Feb 26, 2023

I preferred to not include the PoC here in public.

For one thing, it's quite trivial to construct one from the issue description, and for another, Pygments does not make security guarantees, so this is not a security issue. If it can be used for RE DOS, that's a bug in the application that uses Pygments. If @birkenfeld (i.e. the person behind georg@python.org) replied, I'm sure he told you the same thing.

Should I open another issue here?

Yes, please.

@SCH227
Copy link
Author

SCH227 commented Feb 26, 2023

For one thing, it's quite trivial to construct one from the issue description

It is waay more trivial to copy and paste it.

Pygments does not make security guarantees, so this is not a security issue. If it can be used for RE DOS, that's a bug in the application that uses Pygments.

I understand, my aim is not to bother with which should be the most accurate tag for these kind of bugs or to discuss who should tackle it first. You project is great, and I am very grateful you are maintaining it.
I just want to contribute to make it better, and at the same time warn users of behaviors that may affect them negatively (besides not reading the docs xD). I understand that's the reason why CVE-2021-20270 and CVE-2021-27291 were issued in the past.

Thank you! Let me know if I can be of help issuing a patch and/or testing

@jeanas jeanas closed this as completed in 97eb3d5 Mar 1, 2023
@jeanas
Copy link
Contributor

jeanas commented Mar 1, 2023

Should be better with 97eb3d5. Please confirm.

@SCH227
Copy link
Author

SCH227 commented Mar 4, 2023

LGTM!

chigby added a commit to freedomofpress/pressfreedomtracker.us that referenced this issue Jun 5, 2023
Fixes vulnerability described here:

pygments/pygments#2355
chigby added a commit to freedomofpress/securedrop.org that referenced this issue Jun 5, 2023
I've added pygments directly to requirements.in since we are actually
using this library in our own code.

The version upgrade fixes the vulnerability described here:

pygments/pygments#2355
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug type: a bug
Projects
None yet
Development

No branches or pull requests

2 participants