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

feat(checker): add ngircd checker #3003

Merged
merged 1 commit into from
Jun 6, 2023
Merged

Conversation

ffontaine
Copy link
Contributor

Unfortunately, we can't catch some ngircd version which are on two digits (e.g. 25) because cve-bin-tool only extracts strings which have more than 3 characters

@codecov-commenter
Copy link

codecov-commenter commented May 12, 2023

Codecov Report

Merging #3003 (6740fc3) into main (6e807b1) will decrease coverage by 0.65%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3003      +/-   ##
==========================================
- Coverage   82.92%   82.27%   -0.65%     
==========================================
  Files         674      704      +30     
  Lines       10650    10933     +283     
  Branches     1429     1476      +47     
==========================================
+ Hits         8831     8995     +164     
- Misses       1457     1555      +98     
- Partials      362      383      +21     
Flag Coverage Δ
longtests 81.56% <100.00%> (-0.90%) ⬇️
win-longtests 79.81% <100.00%> (-0.64%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cve_bin_tool/checkers/__init__.py 91.48% <ø> (ø)
cve_bin_tool/checkers/ngircd.py 100.00% <100.00%> (ø)
test/test_data/ngircd.py 100.00% <100.00%> (ø)

... and 50 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor change: I'd just like the comment about detection on 2-digits to go in the checker so it's a bit easier for people to find if it comes up.

We could potentially look into having a 2-character scan happen if and only if the pattern ngIRCd is found. I don't think it would impossible to engineer that somehow by overriding the checker functions to grab the strings, but I don't know off the top of my head how much work it would be. We could also see if enabling 2-character strings in all scans would cause a performance hit or break any existing checkers.


https://www.cvedetails.com/product/4749/Ngircd-Ngircd.html?vendor_id=2709
https://www.cvedetails.com/product/26242/Barton-Ngircd.html?vendor_id=12890

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Note: Unfortunately, we can't catch some ngircd version which are on two digits (e.g. 25)
because cve-bin-tool only extracts strings which have more than 3 characters

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the time-being, I added the comment in the checker as requested

Unfortunately, we can't catch some ngircd version which are on two
digits (e.g. 25) because cve-bin-tool only extracts strings which have
more than 3 characters

Signed-off-by: Fabrice Fontaine <fabrice.fontaine@orange.com>
Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Let's get this merged then, and I'll open a separate issue about the string lengths just so I don't forget about it.

@terriko terriko merged commit d4e85ff into intel:main Jun 6, 2023
21 of 22 checks passed
@ffontaine ffontaine deleted the add-ngircd-checker branch June 7, 2023 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants