-
Notifications
You must be signed in to change notification settings - Fork 506
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
tool includes reportlab dependency which is flagged by Safety #1459
Comments
Yes, there was an issue with reportlab. You should make sure you have upgraded to the latest version of reportlab, which has a partial fix. We implemented the recommended mitigations including validation of the data sent to reportlab and should not be affected, but it you feel that's not true please do feel free to recommend additional security measures. |
Whoops, didn't mean to close that right away in case you want to add additional comments, but I'll probably close it in a week or so if you don't. |
Well, I have the latest from reportlab as it would have been installed as a dependency of this tool. I just find it in general very suspicious that a tool for finding CVEs itself is using a dependency that has an open CVE with no resolution. I was not previously using reportLab and didn't have it installed. Furthermore, how is it that cve-bin-tool is not flagging reportLab itself, as it is included in the CVE databases. https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-28463 |
When I do a check it's reporting correctly:
I don't know how familiar you are with this particular CVE, but it's basically "it is possible to create a pdf that loads external resources" and the response has been "yes, that's a feature, you can disable it in this way" -- there's no fix beyond validation of the input you send to reportlab. I'm honestly a bit surprised it hasn't been actively disputed by the reportlab team. If you don't want to use reportlab or pdf reports, you can uninstall it and still use cve-bin-tool. Some of the tests will fail and obviously pdf export won't work, but it should otherwise function without it. I believe I have an open bug about improving the test and runtime experience if reportlab isn't installed; I'll see if I can find that and prioritize it for the 3.1 release, and if people are uncomfortable with this CVE we could make reportlab an optional dependency going forwards. |
Update: I believe we've verified that everything can run without report lab (although the pdf tests do of course fail). I think we'd still like to have the default version of cve-bin-tool not install reportlab, and have it installed only if the user specifically says they need pdf support or installs it manually themselves. |
This PEP may be relevant: And this: |
I think I added code to detect if reportlab was installed and if it wasn't then prevent pdf from being a valid output option. To make this the default install (i.e. reportlab is not installed) does this just need a change to the requirements.txt? |
I also had a similar query as well, are we looking at simply removing reportlab from our requirements list making the user install it on their own for use? Or having it as an extra package [pdf]? As both would remove the default install of reportlab, difference being how the user actually gets the reportlab dependency if need be. |
I sort of envision both:
Either way, the cve-bin-tool code used should be the same, it's just whether reportlab is installed by pip or not. I don't know how to do this, but it seems possible? |
Could we just remove reportlab from requirements.txt and have a second
requirements.txt (optional-requirements.txt) which contains reportlab and
add some documentation to say if you need pdf support run pip install -r
optional-requirements.txt.
…On Wed, 30 Mar 2022, 18:55 Terri Oda, ***@***.***> wrote:
I sort of envision both:
1.
If the user installs the default version of cve-bin-tool, reportlab
would not be installed. cve-bin-tool can print some messages saying you
need reportlab if you want to enable pdfs after the fact.
2.
I'd like to provide a cve-bin-tool[pdf] option so people can install
it and get reportlab included without having to do an explicit install
themselves.
Either way, the cve-bin-tool code used should be the same, it's just
whether reportlab is installed by pip or not. I don't know how to do this,
but it seems possible?
—
Reply to this email directly, view it on GitHub
<#1459 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACAID2YVBCSYVIVKKIJ7MITVCSIRNANCNFSM5J7AL7JQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Yeah, that'll work. I'll open a PR with that in a few minutes, but then I'm going to do a bit of research and see if I can make it so that |
Having [pdf] should be pretty straight forward, you just need to define a different set of dependencies in setup.py and it should work as expected. |
Okay, PR with some explanatory text and the requirements changes open. If anyone's got time to review my wording, a quick "this sounds ok" or a "maybe we could say..." review would be very much appreciated. #1626 Still digging on making cve-bin-tool[pdf] working |
hah, amusingly the actual example in the docs is pdf support using reportlab: |
Okay, #1626 is ready for full review. The diff in Note that I also reved the version number in here; that was to facilitate some testing against testpypi for package building. |
* fixes #1459 * fix: Remove reportlab from default install * docs: Provide info on how to install cve-bin-tool[PDF]
https://snyk.io/vuln/pip:reportlab
I'm currently using
safety
. I tried adding cve-bin-tool as a dev dependency. When I then ransafety
again on my environment, it reports that reportLab is a vulnerability.https://pypi.org/project/safety/
The text was updated successfully, but these errors were encountered: