-
Notifications
You must be signed in to change notification settings - Fork 465
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
micrium uC/Lib vulnerability causes cve-bin-tool to delete triage response data from triage input file #4417
Comments
That definitely sounds like a bug. @mastersans just did a huge rework of the triage system that's poised to come out in 3.4 : do you mind trying the 3.4rc1 build and seeing if you have the same issue or if he's already fixed that issue? https://pypi.org/project/cve-bin-tool/3.4rc1/ Also, a quick heads up: you're showing as using python2.7 which we stopped supporting when it stopped getting security updates. I doubt this problem is related to that, but I'm honestly a bit surprised that it still works at all in 2.7. I figured by now we'd have used something too new and it would have broken. |
I have Python 3.1.1 also on my computer I can try it with that easily enough first. |
It still happens with python 3.1.1 |
I downloaded cve-bin-tool v3.4rc1 and it seems a few of the parameters/options have changed also.
So then I ran the command like this:
With my full SBOM and cve-bin-tool v3.3 there are 4 products with known CVEs and the last lines of output are:
Please advise as to what else has changed with this version so I can get it to run correctly. |
@mastersans has the new docs here: https://cve-bin-tool.readthedocs.io/en/latest/triaging_process.html |
If you are changing options and taking away support for vex files it sounds like this is a major overhaul and maybe shouldnt just be numbered 3.4 but instead 4.0. |
@tzirn renaming filename.vex to filename.json should work. Thanks |
@mastersans OK I ran the following command:
This completed but did not fix the bug. I attached all the files this run created. The SBOM is the same as what was attached above and the triageFile is the same, just renamed to .json as you mentioned. So you can run it there very easily. Note that the run with the new tool creates 2 "test-triage_out.json" files PLEASE ALSO NOTE: there appears to be a new bug with this test version in that the output report *.csv file no longer has the "comments" column filled out. It used to contain the reasoning given in the analysis --> detail section of the traigefile json/vex - now that column is empty. So no rationale is showing for the analysis stored in the "remarks" column. |
Does the extension have to matter? If renaming works, I feel like we should just allow .vex as a possible extension. |
Let me see what i can find out, my linux machine got froze and won't work after that so i have to setup everything. |
So, I've been thinking about whether we should change the planned release number. I'm not adverse to making this 4.0, but it seems like if we allow .vex and maybe turn the old flag into syntactic sugar for the new one with a deprecation warning, I think we can improve the backwards compatibility, so I'm going to go ahead and see how doable those two things are today. New release is still likely to happen next week for a bunch of non-technical reasons, and I don't think we'll be able to fix everything in this thread (most of this is just going to have to happen in 3.4.1 or 4.0.1) but let me see about smoothing the transition to the new version. |
ok so just to summarize the issues on this thread:
@terriko Considering this 2nd and 3rd issues I can't see how you can put out this version since it losses the triage comments - it makes the output reports useless. See the 2 attachments I put in from my full report with all my SBOM data so you can see where it says its triaged but has no comments. CSV report: Shows #1( last line), #2 (last column) and #3 (see green ?) HTML report 1st capture: shows #2 HTML report 2nd capture: shows #1 (last row) and #3 (see green ?) |
I'm doing the check to see if I need to change the versioning number first because if I switch this to be a 4.0 release it changes some internal process stuff that is easier for me if I know that it needs doing today vs next week. So I'm trying to get it done so I can make appropriate plans.
Sorry about the confusion! |
* related to intel#4417 This adds a little bit of backwards compatibility for vex triage * If --triage-input-file is used, display deprecation warning and convert to --vex-file" so scan can continue. * If file extension in .vex, the first time, make a copy and scan anyhow. On subsequent times, print an error asking the user to use --vex-file <new file name> It might be more elegant to edit lib4vex to handle .vex filenames more seamlessly there, but I feel like "give people one chance and then error" is probably more likely to help people switch to the new arguments so we don't have to support the old ones forever and won't break as many CI jobs. Signed-off-by: Terri Oda <[email protected]>
Okay, backwards compat attempt is up, would love feedback on it: #4421 I need to grab some lunch but I'll continue on to triage comments this afternoon (if @mastersans hasn't already fixed it by then...) |
As I kind of suspected, the problem is that we used to have some magic that turned "detail" into the "comments" inside of comments = strip_remark(vulnerability["analysis"]["detail"]) That got lost in the transition to use lib4vex and so now it's printing something from the actual "comments" field (which is... not usually there). So that answers the why. My lazy attempt just made more blank spaces in the output so I may need to do some explicit plumbing (may need to happen in lib4vex) before I can make it behave as expected. But I thought I'd give an update since I've been doing a lousy job of helping with the actual bug while my brain was pre-release-paperwork mode. |
Oh, I should probably tag @anthonyharrison on this too. I don't have an answer of how to fix it yet. It looks like it's loading the data the way I'd expect from lib4vex: [{'id': 'CVE-2021-26706', parse.py:74
'source-name': 'NVD', 'source-url':
'https://nvd.nist.gov/vuln/detail/CVE-2021-26706', 'description': 'An
issue was discovered in lib_mem.c in Micrium uC/OS uC/LIB 1.38.x and
1.39.00. The following memory allocation functions do not check for
integer overflow when allocating a pool whose size exceeds the address
space: Mem_PoolCreate, Mem_DynPoolCreate, and Mem_DynPoolCreateHW.
Because these functions use multiplication to calculate the pool
sizes, the operation may cause an integer overflow if the arguments
are large enough. The resulting memory pool will be smaller than
expected and may be exploited by an attacker.', 'created':
'NOT_KNOWN', 'updated': '', 'status': 'not_affected', 'comment':
'NotAffected: affects micrium uC/LIB however those functions NOT USED
by Embedded apps', 'remediation': None, 'justification':
'code_not_reachable', 'bom_link':
'urn:cbt:1/micrium#uc\\/lib:1.38.01'}] I think there's possibly an error there that it's "comment" instead of "comments" so that's not helping the propagation, but doing a lazy fix there doesn't seem to be enough. I need to go because my kid is home, so anyone else should feel free to continue from where I'm at if you think you can figure the rest out! Otherwise, I'll be back at it on Monday when I have childcare again (well, barring a bolt of brilliance in the middle of the weekend). |
And also: it looks like our existing vex tests don't have "detail" set on any of them, so we should definitely fix the tests to look for that once we get it fixed because I absolutely do not want another regression here. |
* related to #4417 This adds a little bit of backwards compatibility for vex triage * If --triage-input-file is used, display deprecation warning and convert to --vex-file" so scan can continue. * If file extension in .vex, the first time, make a copy and scan anyhow. On subsequent times, print an error asking the user to use --vex-file <new file name> It might be more elegant to edit lib4vex to handle .vex filenames more seamlessly there, but I feel like "give people one chance and then error" is probably more likely to help people switch to the new arguments so we don't have to support the old ones forever and won't break as many CI jobs. Signed-off-by: Terri Oda <[email protected]>
Update: so we did have comments set in a few of the vex tests, just not in the expected values from the tests where I looked first because of the "comments" vs "comment" issue. So I've fixed that in #4423 and updated the tests, so there will be tests for comments going forwards. So that's once piece waiting on code review but it should be fixed soon. I merged the basic backwards compat stuff too. Not sure we've solved everything yet, but I need to switch to another task for a while so I'm noting where I'm at for now. |
@terriko @mastersans @anthonyharrison if you have a new build now of what you've fixed I can try it out. |
No build yet (because I haven't finished testing comment propagation and I suspect it's not actually fixed yet) but if you want to try it out from main here's the commands git clone https://github.com/intel/cve-bin-tool.git
cd cve-bin-tool
python -m cve_bin_tool.cli <scan options go here> If you're using a virtualenv for one of your python versions don't forget to activate it. I'm assuming since you tried the release candidates you've got the pre-reqs installed but if you're doing it in a fresh environment you can install them using `pip install -U -r requirements.txt" after the cd step. I'll try to get a build out once I think it's actually working, but it may not be today because something else has come up. |
Okay, managed to find a few minutes to do some very manual tests and the comment propagation seems to be working (for anything other than Threw up a quick rc2 build for you to try here: https://pypi.org/project/cve-bin-tool/3.4rc2/ so you don't have to follow the instructions above. Let me know how that works for you! |
@terriko Is there another way to contact you so we dont keep cluttering up this thread with other issues? this test version 3.4rc2 outputs the reports but crashes before making the triage-output file. However, even though the reports now have the comments from the "details" JSON field back in them, they are missing the "Justification" field string in the beginning of it. I'd rather not keep doing this on this thread otherwise my original bug will get lost in the mix. |
There's lots of ways to contact me, but tracking triage problems in an issue in this repo likely makes the most sense, especially if anyone else testing the pre-release has similar problems they can chime in. Let's open a separate issue for the crash and one for the justification details (though the latter is probably a lib4vex issue so we may need to make that fix in https://github.com/anthonyharrison/lib4vex/ and move an issue there, but we can start with it here and potentially have a workaround in while we figure things out.) |
New issues are here:
I've got a fix for the 1st one merged and a fix for the second is running through the CI system now. |
Well, no 3.4 release for me this week after all, but I have a new 3.4rc3 build for you to try! Assuming nothing in rc3 explodes, hopefully the final release will be out next week. I'm going to work on some tests and then hopefully come back to the original issue with uC/Lib after those are in place. |
Quick heads up: since I didn't hear anything from you and all other feedback from the release candidates has been positive, I've gone ahead and put out the 3.4 release: https://pypi.org/project/cve-bin-tool/3.4/ I'm going to tag this particular bug as "future" so it will get moved into 3.4.1 planning, but I'm going to be busy with some other stuff starting this week so 3.4.1 development probably won't kick off right away. If you want to contact me outside of the public issues for offline feedback (as some of the other folk testing the release candidates have done since the feedback was "it worked for us" rather than a bug that needed filing), feel free to email me at my intel email listed in my github profile. |
sorry I have been very busy with my job this week. I will test this out tomorrow AM as it's almost the end of my day now and I have more meetings. |
@terriko I am working on a different computer today so I installed Python 3.8.0 and the new version of cve-bin-tool that you linked above. I can't get it to run at all as Python crashes. So I dont know if it's the new install of python or the cve-bin-tool. I'll have to try again tomorrow on my original computer if I can't figure this out today. |
@terriko please verify the parameters in this screen shot ( I copied my original *.vex triage file to *.json as previously instructed) |
That looks like a problem with your python install or operating system rather than cve-bin-tool. Usually we get at least the initial version and nvd download information before there's even a chance of the OS halting the process. Are you on windows? We no longer support python 3.8 on windows (there's an issue with the tarfile library that we couldn't find an adequate workaround for so we can't run the tests) and it's going out of support in general on October 31 this year. Before you spend too much time debugging your issue, I highly recommend you switch to python 3.12 which is the only version on windows we can currently test. |
ok I updated to Python 3.8 because when I ran the tool install using 3.7 the install reported that version of cve-bin-tool required 3.8. I can update to 3.12 and try again |
@terriko OK I ran it on my main computer today. Below are the results I got from running 3.4 with Python v3.12.0: 1. 3.4's CSV report removes/doesnt report CVE-2020-27209 (micro-ecc_project) , even though micro-ecc is in my SBOM and that CVE is in my triage input file - it just doesnt list it at all in the output report. This wont let me attach an HTML file so I can't attach that report. Here are the rest of the files used or created from that command: test_SBOM.csv |
@tzirn the problem regarding first 2 arise due to the reason that the sbom used have micro-ecc 's version set to 1 instead it should be 1.0, you can try change the version to 1.0 and it will fix the issue for reference currently cve-bin-tool checks weather the data that is obtain from scanning vex file is also present in the main file being scanned in this case its a sbom, if its not present then the tool logs out a message that it didn't find product in the main file, Is a valid file being scanned, its mostly to avoid wrong vex file being scanned since we don't have a for sure way to tell that a main file is linked with a vex file or not, for cyclonedx we do support bom-link to check if the cyclonedx vex is linked with correct cyclonedx sbom or not. heres also the output of html when version 1.0 is used instead of 1: |
@mastersans I put 1.0 in the csv and then when I save it excel changes it to 1. However, this worked with version 3.3 so it should still work in v3.4 why change it when Excel fights the 1.0 and changes it to 1? Other people will have this issue and since it worked with 3.3 (I haven't changed the SBOM at all) it seems like it was accidental so a bug. |
I suspect what's happening with the 1.0 vs 1 issue is that we're checking for an exact match in that case since it's not a range comparison and since it's not an exact match it's not coming back. I'll have to think a little bit about how we want to handle that more correctly. What it's doing is... technically correct but likely unexpected in this case and maybe similar ones. For the case of a version 1 vs 1.0 we could just try adding .0 on the end and compare a second time, but for some versions (particularly those that are dates rather than semantic versions) that might be hideously incorrect. If you get a chance, we should move just the 1 vs 1.0 part to a separate issue for tracking. (If you don't get a chance, I'll split that off next week when I do my Monday triage/PR review but if you want to track the fix you'll have to manually follow it then.) |
(And I wish I could say "excel's bugs are not my problem" but I think a lot of people probably use excel to edit .csv files so I'd like to at least see how viable it is to handle this on our end. Though seriously, WTF excel you are the worst and this is why I stopped using you for scientific analysis when I was that kind of scientist.) |
I opened 4467 And just wanted to say I get what you're saying but my point is this used to work in v3.3 so then it should still work in v3.4. If it never worked and was documented that 1 had to be forced to 1.0 then that would have been fine. But since it used to work I thought it should still work, rather than remove functionality. |
Hi - just checking back in on this original issue. If there is anyone looking into the original issue please let me know if I can help in any way. |
I don't think anyone's looking into it this week. I've been dragged into a more urgent project for the moment. |
Random thought: is this happening only with |
We have 2 Micrium uC items in our SBOM listed in the official-cpe-dictionary_v2.3.xml file. Micrium uC/OS and uC/Lib. The uC/OS doesn't have a \ in the product name (see below). This is the Micrium product data we have in our SBOM.csv file:
|
Hi just checking back in to see if anyone is looking into this? Thanks for anyone trying! |
Description
cve-bin-tool deletes triage analysis/response to micrium uC/Lib vulnerability. We have micrium uC/Lib listed in our CSV SBOM and there is 1 vulnerability for micrium uC/lib. cve-bin-tool finds it and we have it output the vulnerabilities into a vex file. We then add analysis/triage responses within the vex file to all the vulnerabilities found. When I re-run the cve-bin-tool with that vex file as input it uses all the responses correctly in the output report except for the uC/Lib one - it always deletes/ignores the data I've added and lists it as "unexplored" in the remarks field of the output report. The rest are correct.
To reproduce
Steps to reproduce the behavior:
vendor: micrium; product: uc/lib; version 1.38.01; name: micrium uC/OS Lib; Unique Identifier: cpe:2.3:a:micrium:uc/lib:1.38.00:::::::*; type: library; relationship: included in app; URL: https://www.silabls.com/developers/micrium-os;
cve-bin-tool -i test_SBOM.csv --triage-input-file test-triageFile.vex -f html,csv --vex test-triage_out.vex -o vulnerability-report-out
Expected behaviour: The output report files should list CVE-2021-26706 as triaged with the data put in the triage input file
Actual behaviour: The output report files list CVE-2021-26706 as unexplored
Version/platform info
Version of CVE-bin-tool( e.g. output of
cve-bin-tool --version
): 3.3Installed from pypi or github? pypi
Operating system: Linux/Windows
systeminfo | findstr /B /C:"OS Name" /C:"OS Version"
OS Name: Microsoft Windows 10 Pro
OS Version: 10.0.19042 N/A Build 19042
Python version (e.g.
python3 --version
): python v2.7Running in any particular CI environment we should know about? (e.g. Github Actions) None
Anything else?
Feel free to add any other context here.
see attached example SBOM, and output csv file and a screen shot of the vex file input vs vex file output (jpg) (I can't attach the triage vex input or output files or the HTML file - your tool doesnt support those types )
test_SBOM.csv
vulnerability-report-out.csv
**Contents of test-triageFile.vex INPUT file **
{
"bomFormat": "CycloneDX",
"specVersion": "1.4",
"version": 1,
"vulnerabilities": [
{
"id": "CVE-2021-26706",
"source": {
"name": "NVD",
"url": "https://nvd.nist.gov/vuln/detail/CVE-2021-26706"
},
"ratings": [
{
"source": {
"name": "NVD",
"url": "https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?name=CVE-2021-26706&vector=CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H&version=3.1"
},
"score": 9.8,
"severity": "critical",
"method": "CVSSv3",
"vector": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H"
}
],
"description": "An issue was discovered in lib_mem.c in Micrium uC/OS uC/LIB 1.38.x and 1.39.00. The following memory allocation functions do not check for integer overflow when allocating a pool whose size exceeds the address space: Mem_PoolCreate, Mem_DynPoolCreate, and Mem_DynPoolCreateHW. Because these functions use multiplication to calculate the pool sizes, the operation may cause an integer overflow if the arguments are large enough. The resulting memory pool will be smaller than expected and may be exploited by an attacker.",
"recommendation": "",
"advisories": [],
"created": "NOT_KNOWN",
"published": "NOT_KNOWN",
"updated": "",
"analysis": {
"state": "not_affected",
"response": [
"code_not_reachable"
],
"detail": "NotAffected: affects micrium uC/LIB however those functions NOT USED by Embedded apps",
"justification": "code_not_reachable"
},
"affects": [
{
"ref": "urn:cbt:1/micrium#uc\/lib:1.38.01"
}
]
}
]
}
The text was updated successfully, but these errors were encountered: