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

auxiliary/fileformat/badpdf: fix syntax and logic error in options handling #11217

Merged
merged 1 commit into from Jan 10, 2019

Conversation

Projects
None yet
3 participants
@cnotin
Copy link
Contributor

cnotin commented Jan 9, 2019

Fixes a logic error in the handling of null options.
Related to @bcoles comment in original PR: #10148 (comment)

Before patch

Setup:

msf auxiliary(fileformat/badpdf) > options 

Module options (auxiliary/fileformat/badpdf):

   Name       Current Setting  Required  Description
   ----       ---------------  --------  -----------
   FILENAME                    no        Filename
   LHOST      127.0.0.1        yes       Host listening for incoming SMB/WebDAV traffic
   PDFINJECT                   no        Path and filename to existing PDF to inject UNC link code into

FILENAME and PDFINJECT empty, it should say to fill one but instead it complains about the extension:

msf auxiliary(fileformat/badpdf) > run

[-] FILENAME or PDFINJECT must end with '.pdf' file extension
[*] Auxiliary module execution completed

Same message when the extension is missing, but it's correct this time:

msf auxiliary(fileformat/badpdf) > set FILENAME a
FILENAME => a
msf auxiliary(fileformat/badpdf) > run

[-] FILENAME or PDFINJECT must end with '.pdf' file extension
[*] Auxiliary module execution completed

It works when everything is good:

msf auxiliary(fileformat/badpdf) > set FILENAME a.pdf
FILENAME => a.pdf
msf auxiliary(fileformat/badpdf) > run

[+] a.pdf stored at /root/.msf4/local/a.pdf
[*] Auxiliary module execution completed

After patch

Same setup:

msf auxiliary(fileformat/badpdf) > options 

Module options (auxiliary/fileformat/badpdf):

   Name       Current Setting  Required  Description
   ----       ---------------  --------  -----------
   FILENAME                    no        Filename
   LHOST      127.0.0.1        yes       Host listening for incoming SMB/WebDAV traffic
   PDFINJECT                   no        Path and filename to existing PDF to inject UNC link code into

When both FILENAME and PDFINJECT are empty, it says to configure one:

msf auxiliary(fileformat/badpdf) > run

[-] Please configure either FILENAME or PDFINJECT
[*] Auxiliary module execution completed

If one is configured, but with missing extension, it says to add it:

msf auxiliary(fileformat/badpdf) > set FILENAME a
FILENAME => a
msf auxiliary(fileformat/badpdf) > run

[-] FILENAME or PDFINJECT must end with '.pdf' file extension
[*] Auxiliary module execution completed

Still works fine when everything is good:

msf auxiliary(fileformat/badpdf) > set FILENAME a.pdf
FILENAME => a.pdf
msf auxiliary(fileformat/badpdf) > run

[+] a.pdf stored at /root/.msf4/local/a.pdf
[*] Auxiliary module execution completed

Syntax fix

This also fixes a syntax error with single-quotes used in a single-quotes string which triggers:

[-] Auxiliary failed: NoMethodError undefined method `pdf' for "FILENAME or PDFINJECT must end with ":String
[-] Call stack:
[-]   /usr/share/metasploit-framework/modules/auxiliary/fileformat/badpdf.rb:49:in `run'
@cnotin

This comment has been minimized.

Copy link
Contributor

cnotin commented Jan 9, 2019

Cc @rmdavy (module author)

@bcoles bcoles added module bug labels Jan 9, 2019

@busterb busterb self-assigned this Jan 10, 2019

@busterb busterb merged commit cf1b4b4 into rapid7:master Jan 10, 2019

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

busterb added a commit that referenced this pull request Jan 10, 2019

@busterb

This comment has been minimized.

Copy link
Contributor

busterb commented Jan 10, 2019

Thanks @cnotin

@busterb

This comment has been minimized.

Copy link
Contributor

busterb commented Jan 10, 2019

Release Notes

This fixes issues handling error conditions in the badpdf local exploit module.

msjenkins-r7 added a commit that referenced this pull request Jan 10, 2019

@cnotin cnotin deleted the cnotin:patch-2 branch Jan 10, 2019

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