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

Pants uses a version of ujson that is susseptable to buffer overflow #15143

Closed
asherf opened this issue Apr 14, 2022 · 4 comments
Closed

Pants uses a version of ujson that is susseptable to buffer overflow #15143

asherf opened this issue Apr 14, 2022 · 4 comments
Labels

Comments

@asherf
Copy link
Member

asherf commented Apr 14, 2022

https://nvd.nist.gov/vuln/detail/CVE-2021-45958
ujson is dependent upon via the seemingly unmaintained python-lsp-jsonrpc (added in #14329) - last commit in January 2022, but almost no commits in 2021.

Screen Shot 2022-04-14 at 10 55 15 AM

Hopefully this can be closed when a patched version of ujson is published (it seems to be more active).

@tdyas @stuhood fyi

@asherf asherf added the bug label Apr 14, 2022
@tdyas
Copy link
Contributor

tdyas commented Apr 14, 2022

I would be fine with us just vendoring python-lsp-jsonrpc and removing the ujson dependency.

Thoughts?

@tdyas
Copy link
Contributor

tdyas commented Apr 14, 2022

Although given the usage is in the BSP server, an attacker would already have control of the system to be able to invoke Pants on the system (since the BSP server listens on stdin/stdout). So the attacker can already do whatever they want.

@asherf
Copy link
Member Author

asherf commented Apr 14, 2022

it looks like ujson has already addressed the issue: ultrajson/ultrajson#519
I suggest waiting a few days for them to make a release and then since there is no version constraint on python-lsp-jsonrpc wrt ujson see: https://github.com/python-lsp/python-lsp-jsonrpc/blob/1c97c6da4778252b4e49f03ee6aba778b7e7092e/setup.py#L41
re-generating the lock file will solve the issue.

I mostly opened this issue so we can track that.
I don't think vendoring/forking makes sense unless this doesn't solve in the next week or so.
I agree that in this context this is low risk.

@stuhood
Copy link
Sponsor Member

stuhood commented Jan 20, 2023

Now using 5.7.0

@stuhood stuhood closed this as completed Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants