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

Scripted update to add missing example attributes #400

Merged
merged 2 commits into from
Jan 28, 2022

Conversation

dabdine
Copy link
Contributor

@dabdine dabdine commented Jan 26, 2022

Description

Adds missing example attributes to <example> elements in recog fingerprint XML files, greatly reducing recog_verify warnings.

This is only round 1. Subsequent rounds will target some hand modifications to target fingerprints missing examples altogether. Additionally, there is one warning that is not fixed with this patch due to it being an edge case: The fingerprint regex conditionally matches a version, but the only example does not supply one. That will be fixed by hand as well in a subsequent PR.

The script that was used to make these changes is here:
https://gist.github.com/dabdine/6c641407e805ab33530ec67459a94c3a

Motivation and Context

Clean up warnings so that more strict enforcement of quality can eventually be turned on.

How Has This Been Tested?

Ran recog_verify

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have updated the documentation accordingly (or changes are not required).
  • I have added tests to cover my changes (or new tests are not required).
  • All new and existing tests passed.

@dabdine
Copy link
Contributor Author

dabdine commented Jan 27, 2022

Fails for golang, works for C/JRuby & Java. The failing cases are because of captured junk text in a service version. I'll tweak the regex to see if I can get the behavior to work consistently across go/ruby/Java.

@@ -28,7 +28,7 @@

<fingerprint pattern="^(?i:(?:Microsoft )?Windows 10 Mobile(?:\s([a-z]+))?(?: Edition)?)$">
<description>Windows 10 Mobile</description>
<example os.product="Windows 10 Mobile">Windows 10 Mobile Edition</example>
<example os.product="Windows 10 Mobile" os.edition="Edition">Windows 10 Mobile Edition</example>
Copy link
Contributor Author

@dabdine dabdine Jan 28, 2022

Choose a reason for hiding this comment

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

This looks buggy--should be "Mobile," but it's a current shipping defect. This patch only makes it obvious since it interpolates the field value from the match groups derived from the example string.

<example>Qpopper (version 4.0.3) at domain starting. &lt;xxx@domain&gt;</example>
<example service.version="4.0.16" host.domain="foo.example.com">Qpopper (version 4.0.16) at foo.example.com</example>
<example service.version="2.53" host.domain="domain starting. &lt;xxx@domain&gt;">QPOP (version 2.53) at domain starting. &lt;xxx@domain&gt;</example>
<example service.version="4.0.3" host.domain="domain starting. &lt;xxx@domain&gt;">Qpopper (version 4.0.3) at domain starting. &lt;xxx@domain&gt;</example>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bugged regex

@@ -112,7 +112,7 @@

<fingerprint pattern="^Microsoft Exchange 2000 POP3 server version (\d+\.\d+\.\d+\.\d+) (.+) ready.$">
<description>Microsoft Exchange Server 2000</description>
<example>Microsoft Exchange 2000 POP3 server version 6.0.6603.0 (host) ready.</example>
<example service.version="6.0.6603.0" host.name="(host)">Microsoft Exchange 2000 POP3 server version 6.0.6603.0 (host) ready.</example>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bugged, no parens should exist in host.name

@dabdine
Copy link
Contributor Author

dabdine commented Jan 28, 2022

Regarding the comments about bugs above, I'm simply noting them for some hand mods later. I don't intend to fix those as part of this (automated) patch

@mkienow-r7
Copy link
Contributor

Regarding the comments about bugs above, I'm simply noting them for some hand mods later. I don't intend to fix those as part of this (automated) patch

Do you intend to make those fixes or is that an exercise left to the reader? :)

Is there a reason for not making those corrections as part of this PR or a PR against your PR branch?

@dabdine
Copy link
Contributor Author

dabdine commented Jan 28, 2022

@mkienow-r7 i'm open to suggestions! I was going to fix some of these today assuming this patch lands. I think it'd be better to isolate the hand-crafted fixes from this patch though and do it post-merge

<example hw.product="Q7406">AXIS Q7406 Video Encoder Blade 5.01 (Aug 01 2008) ready.</example>
<example hw.product="241Q">AXIS 241Q Video Server 4.47.2 (Dec 11 2008) ready.</example>
<example hw.version="5.07.2">AXIS P7701 Video Decoder 5.07.2 (Apr 20 2010) ready.</example>
<example hw.product="Q7406" hw.version="Blade">AXIS Q7406 Video Encoder Blade 5.01 (Aug 01 2008) ready.</example>
Copy link
Contributor

Choose a reason for hiding this comment

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

Marking this for follow up: Incorrect capture by the existing regex.

<example os.product="2200">HP LaserJet 2200</example>
<example os.product="4050">LASERJET 4050</example>
<example os.product="4 PLUS">LASERJET 4 PLUS</example>
<example os.product="Professional P1606dn">HP LaserJet Professional P1606dn</example>
Copy link
Contributor

Choose a reason for hiding this comment

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

Marking this for follow up PR: This should be hw.product not os.product

<example os.product="designjet 110plus">hp designjet 110plus</example>
<example os.product="DESIGNJET 1050C">DESIGNJET 1050C</example>
<example os.product="DESIGNJET 1055CM">DESIGNJET 1055CM</example>
<example os.product="DESIGNJET 700">DESIGNJET 700</example>
Copy link
Contributor

@tsellers-r7 tsellers-r7 Jan 28, 2022

Choose a reason for hiding this comment

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

Marking this for follow up PR: This should be hw.product not os.product

Similar elsewhere in this file.

<example>3COM VCX Connect 200</example>
<example>3COM VCX Server</example>
<example>3COM VCX V7005</example>
<example os.product="VCX Connect 100">3COM VCX Connect 100</example>
Copy link
Contributor

@tsellers-r7 tsellers-r7 Jan 28, 2022

Choose a reason for hiding this comment

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

Marking this for follow up PR: Should be hw.product
Similar below.

<example>CX400 - Flare 2.19.0.400.5.040</example>
<example>CX500 - Flare 2.07.0.500.5.020</example>
<example>CX700 - Flare 2.24.0.700.5.022</example>
<example os.product="CX3-10c" os.version="3.26.0.10.5.032">CX3-10c - Flare 3.26.0.10.5.032</example>
Copy link
Contributor

Choose a reason for hiding this comment

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

Marking this for follow up PR: Should be hw.product

Ugh, sooooooo much of this.

@mkienow-r7
Copy link
Contributor

mkienow-r7 commented Jan 28, 2022

@dabdine It seems I had things a little mixed up from the verify workflow failure the other day. I agree, and I'm fine with collaborative post-merge corrections.

@mkienow-r7 mkienow-r7 merged commit f32f3a4 into rapid7:master Jan 28, 2022
@mkienow-r7
Copy link
Contributor

It's really nice to see the warning count drop dramatically. Thank you for the contribution @dabdine!

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.

3 participants