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

Fix examples in architecture.xml, ftp_banners.xml, and ssh_banners.xml #406

Merged
merged 4 commits into from
Feb 1, 2022

Conversation

dabdine
Copy link
Contributor

@dabdine dabdine commented Feb 1, 2022

Description

Fixes fingerprint examples & regular expression issues in ftp_banners.xml, architecture.xml, and ssh_banners.xml. These files no longer yield any warnings whatsoever.

Motivation and Context

Eliminate more warnings & fix bad regular expressions in some fingerprints.

How Has This Been Tested?

bundle exec rake tests

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.

<description>Axis Video encoders/servers</description>
<example hw.product="Q7406" hw.version="Blade">AXIS Q7406 Video Encoder Blade 5.01 (Aug 01 2008) ready.</example>
<example hw.product="Q7406" hw.version="5.01">AXIS Q7406 Video Encoder Blade 5.01 (Aug 01 2008) 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, unexpected capture due to optional string ordering. That's an interesting catch, thanks!

@@ -241,19 +243,6 @@ example.com FTP server (Version: Mac OS X Server) ready.</example>
<param pos="1" name="host.name"/>
</fingerprint>

<fingerprint pattern="^ProFTPD (\d+\.[^\s]+) Server \(Linksys(.*)\) \[(.+)\]$">
Copy link
Contributor Author

@dabdine dabdine Feb 1, 2022

Choose a reason for hiding this comment

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

This looks like a duplicate of what's above. All the examples I've found start with W (WRTxxx). Also, it fails if I keep it since the example in this one was matching the earlier fingerprint and causing bundle exec rake tests to fail.

Copy link
Contributor

@tsellers-r7 tsellers-r7 Feb 1, 2022

Choose a reason for hiding this comment

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

The regex in both of the fingerprints remains unchanged since the initial commit 8 years ago. As you mentioned the only real difference is the W in the regex.

I've checked some of our datasets (all ports, going back years) and I do NOT see any matches for this fingerprint in them.
I DO see matches for the W variant.

++ to seeing this FP removed.

@@ -211,6 +211,8 @@ example.com FTP server (Version: Mac OS X Server) ready.</example>
<param pos="0" name="os.vendor" value="Linksys"/>
<param pos="0" name="os.device" value="WAP"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is os.device a thing? I thought this was hw.device, but I see os.device everywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is a thing but it is used incorrectly more often than not. Also, in almost every case it should be paired with hw.device

Copy link
Contributor Author

Choose a reason for hiding this comment

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

any suggestions on what to do with this? You marked a few hw.device etc. in #400 that need to be corrected. We probably just need to audit the file specifically for all of those.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd add hw.device to this FP but honestly we need to perform an audit and fix them so either way is fine.

@dabdine
Copy link
Contributor Author

dabdine commented Feb 1, 2022

Added one fix for ssh_servers.xml as well, but this file still has warnings to address.

xml/ftp_banners.xml Outdated Show resolved Hide resolved
@dabdine dabdine changed the title Fix examples in architecture.xml and ftp_banners.xml Fix examples in architecture.xml, ftp_banners.xml, and ssh_banners.xml Feb 1, 2022
@dabdine
Copy link
Contributor Author

dabdine commented Feb 1, 2022

I went ahead and remediated all remaining SSH examples in ssh_banners.xml. This PR now closes out all issues on the three mentioned files.

@mkienow-r7 mkienow-r7 merged commit 9ff706f into rapid7:master Feb 1, 2022
@mkienow-r7
Copy link
Contributor

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