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 TypeError in sessions -v and add an "unknown" platform class #10773

Merged
merged 4 commits into from Oct 8, 2018

Conversation

Projects
None yet
2 participants
@h00die
Contributor

h00die commented Oct 8, 2018

Fixes #10772

To test this you'll need to edit ssh_login to return nil since I believe this is an edge case.

After patch:

msf5 auxiliary(scanner/ssh/ssh_login) > sessions -v

Active sessions
===============

  Session ID: 1
        Name: 
        Type: shell 
        Info: SSH cisco:cisco (2.2.2.2:22)
      Tunnel: ?? -> ?? (2.2.2.2)
         Via: auxiliary/scanner/ssh/ssh_login
   Encrypted: false
        UUID: 
     CheckIn: <none>
  Registered: No

While this fixes the nil concat issue, @wvu-r7 @bcook-r7 any opinion on what a case select default value should be on ssh_login to prevent a nil return? Empty string or 'Unknown' seem logical to me, (and I can add that on to this PR, just wanted to get more opinions).

@h00die h00die added the bug label Oct 8, 2018

@wvu-r7

This comment has been minimized.

Show comment
Hide comment
@wvu-r7

wvu-r7 Oct 8, 2018

Contributor

Since this is the text output library, I think something like "unknown" is just fine. There is currently no constant class for it.

Contributor

wvu-r7 commented Oct 8, 2018

Since this is the text output library, I think something like "unknown" is just fine. There is currently no constant class for it.

@wvu-r7 wvu-r7 self-assigned this Oct 8, 2018

@h00die

This comment has been minimized.

Show comment
Hide comment
@h00die

h00die Oct 8, 2018

Contributor

I was thinking of adding on to

so its more 'official' and leaving the rank to be the default of 0

Contributor

h00die commented Oct 8, 2018

I was thinking of adding on to

so its more 'official' and leaving the rank to be the default of 0

@wvu-r7

This comment has been minimized.

Show comment
Hide comment
@wvu-r7

wvu-r7 Oct 8, 2018

Contributor

Sounds good to me!

Contributor

wvu-r7 commented Oct 8, 2018

Sounds good to me!

@h00die

This comment has been minimized.

Show comment
Hide comment
@h00die

h00die Oct 8, 2018

Contributor

Case unknown:

msf5 auxiliary(scanner/ssh/ssh_login) > run

[+] 2.2.2.2:22 - Success: 'cisco:cisco' '  Line has invalid autocommand "id  "'
[*] Command shell session 1 opened (?? -> ??) at 2018-10-08 13:05:09 -0400
[*] Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed
^[[Amsf5 auxiliary(scanner/ssh/ssh_login) > sessions -v

Active sessions
===============

  Session ID: 1
        Name: 
        Type: shell unknown
        Info: SSH cisco:cisco (2.2.2.2:22)
      Tunnel: ?? -> ?? (2.2.2.2)
         Via: auxiliary/scanner/ssh/ssh_login
   Encrypted: false
        UUID: 
     CheckIn: <none>
  Registered: No

also added a new when regex to capture this IOS device as well:

msf5 auxiliary(scanner/ssh/ssh_login) > run

[+] 2.2.2.2:22 - Success: 'cisco:cisco' '  Line has invalid autocommand "id  "'
^[[A^[[A[*] Command shell session 1 opened (?? -> ??) at 2018-10-08 13:06:42 -0400
[*] Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed
^[[Amsf5 auxiliary(scanner/ssh/ssh_login) > sessions -v

Active sessions
===============

  Session ID: 1
        Name: 
        Type: shell cisco-ios
        Info: SSH cisco:cisco (2.2.2.2:22)
      Tunnel: ?? -> ?? (2.2.2.2)
         Via: auxiliary/scanner/ssh/ssh_login
   Encrypted: false
        UUID: 
     CheckIn: <none>
  Registered: No
Contributor

h00die commented Oct 8, 2018

Case unknown:

msf5 auxiliary(scanner/ssh/ssh_login) > run

[+] 2.2.2.2:22 - Success: 'cisco:cisco' '  Line has invalid autocommand "id  "'
[*] Command shell session 1 opened (?? -> ??) at 2018-10-08 13:05:09 -0400
[*] Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed
^[[Amsf5 auxiliary(scanner/ssh/ssh_login) > sessions -v

Active sessions
===============

  Session ID: 1
        Name: 
        Type: shell unknown
        Info: SSH cisco:cisco (2.2.2.2:22)
      Tunnel: ?? -> ?? (2.2.2.2)
         Via: auxiliary/scanner/ssh/ssh_login
   Encrypted: false
        UUID: 
     CheckIn: <none>
  Registered: No

also added a new when regex to capture this IOS device as well:

msf5 auxiliary(scanner/ssh/ssh_login) > run

[+] 2.2.2.2:22 - Success: 'cisco:cisco' '  Line has invalid autocommand "id  "'
^[[A^[[A[*] Command shell session 1 opened (?? -> ??) at 2018-10-08 13:06:42 -0400
[*] Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed
^[[Amsf5 auxiliary(scanner/ssh/ssh_login) > sessions -v

Active sessions
===============

  Session ID: 1
        Name: 
        Type: shell cisco-ios
        Info: SSH cisco:cisco (2.2.2.2:22)
      Tunnel: ?? -> ?? (2.2.2.2)
         Via: auxiliary/scanner/ssh/ssh_login
   Encrypted: false
        UUID: 
     CheckIn: <none>
  Registered: No
Show resolved Hide resolved lib/msf/core/module/platform.rb

h00die added some commits Oct 8, 2018

@wvu-r7

wvu-r7 approved these changes Oct 8, 2018

@wvu-r7 wvu-r7 changed the title from to_s platform to Check if session.platform is nil in the readable text serializer Oct 8, 2018

@wvu-r7 wvu-r7 merged commit edea3c4 into rapid7:master Oct 8, 2018

3 checks passed

Metasploit Automation - Sanity Test Execution Successfully completed all tests.
Details
Metasploit Automation - Test Execution Successfully completed all tests.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

wvu-r7 added a commit that referenced this pull request Oct 8, 2018

Land #10773, session.platform TypeError fix
This also adds an "unknown" platform class.
@wvu-r7

This comment has been minimized.

Show comment
Hide comment
@wvu-r7

wvu-r7 Oct 8, 2018

Contributor

@h00die: See the final changes here: ccfdfb6. Thank you.

Contributor

wvu-r7 commented Oct 8, 2018

@h00die: See the final changes here: ccfdfb6. Thank you.

msjenkins-r7 added a commit that referenced this pull request Oct 8, 2018

Land #10773, session.platform TypeError fix
This also adds an "unknown" platform class.
@wvu-r7

This comment has been minimized.

Show comment
Hide comment
@wvu-r7

wvu-r7 Oct 8, 2018

Contributor

Release Notes

This fixes a TypeError crash in the sessions -v command for an unknown platform. A new "unknown" platform class has been added for future cases where a platform is truly unknown.

Contributor

wvu-r7 commented Oct 8, 2018

Release Notes

This fixes a TypeError crash in the sessions -v command for an unknown platform. A new "unknown" platform class has been added for future cases where a platform is truly unknown.

@wvu-r7 wvu-r7 changed the title from Check if session.platform is nil in the readable text serializer to Fix TypeError in sessions -v and add an "unknown" platform class Oct 8, 2018

@h00die h00die deleted the h00die:nil_platform branch Oct 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment