-
Notifications
You must be signed in to change notification settings - Fork 14k
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 a stack trace in forge_ticket when SPN is blank #18287
Fix a stack trace in forge_ticket when SPN is blank #18287
Conversation
@@ -108,8 +108,6 @@ def forge_ccache(sname:, flags:) | |||
end | |||
|
|||
def forge_silver | |||
raise Msf::OptionValidateError, 'SPN' if datastore['SPN'].blank? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a more correct fox for this is
raise Msf::OptionValidateError, ['SPN'] if datastore['SPN'].blank?
I'm not sure how this got in as is that's my bad, but I do think we want to raise a validiation error if you try to create a silver ticket without an SPN?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An error is still raised, but now it's from the fail_with(Failure::BadConfig, ...
in #validate_spn!
. Does that work, or should it be Msf::OptionValidateError
specifically? What I liked about the Failure::BadConfig
is that it's what the other #validate_
methods use and it also included details as to how it should be formatted.
This is the error now:
msf6 auxiliary(admin/kerberos/forge_ticket) > rerun
[*] Reloading module...
[-] Auxiliary aborted due to failure: bad-config: Invalid SPN, must be in the format <service class>/<host><realm>:<port>/<service name>. Ex: cifs/host.realm.local
[*] Auxiliary module execution completed
msf6 auxiliary(admin/kerberos/forge_ticket) >
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uhhh yea actually that's fine, at the time I wanted there to be a different error message for not setting it vs setting it incorrectly but you're right might as well just output the better message
Nice fix @zeroSteiner! Testing checks out 👌 Before
After
|
Release NotesThis PR fixes a stack trace thrown by the forge_ticket module when the SPN datastore option was left blank. The module now fails due to bad-config and gives a detailed error message. |
This fixes a stack trace in
auxiliary/admin/kerberos/forge_ticket
that would occur when you attempt to forge a silver ticket with a blank SPN.Original stack trace:
To reproduce this. Use the
auxiliary/admin/kerberos/forge_ticket
module. Set theUSER
,DOMAIN
andDOMAIN_SID
options then run theFORGE_SILVER
action. The text in the exception that's raised when validating the SPN was changed as well.Ex:
was presumably short for "example" so it should be an example of a legitimate SPN. It has been updated to the example value used in the docs for clarity. TheDOMIN_SID
example, is already the value from the docs.Testing
auxiliary/admin/kerberos/forge_ticket
moduleUSER
,DOMAIN
andDOMAIN_SID
optionsFORGE_SILVER
action