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

[enhancement] Adding column to show if SPN exists in finddelegations.py #1727

Merged
merged 2 commits into from
May 23, 2024

Conversation

p0dalirius
Copy link
Contributor

Hi,

I have added a new column "SPN Exists" to easily see if SPN Jacking is possible.

Here is a redacted example of output:
image

Best regards,

@anadrianmanrique anadrianmanrique added the in review This issue or pull request is being analyzed label Apr 18, 2024
@gabrielg5
Copy link
Collaborator

Hi @p0dalirius,

Checking your changes I found a path that is triggering an error when generating the output.

This line is not setting any value to the new column

answers.append([rights, objType, 'Resource-Based Constrained', sAMAccountName])

which is resulting in an exception iterating rows to draw the table

def printTable(items, header):
colLen = []
for i, col in enumerate(header):
rowMaxLen = max([len(row[i]) for row in items])
colLen.append(max(rowMaxLen, len(col)))

Also, could you revert those quoting changes? that makes the PR more atomic and helps validate what is the real change

thank you!!

@gabrielg5 gabrielg5 added the waiting for response Further information is needed from people who opened the issue or pull request label May 17, 2024
@p0dalirius
Copy link
Contributor Author

Hi @gabrielg5,

I have fixed the code path

I have reverted the quoting changes, although the code really should be double quoted and not single quoted everywhere.

Best regards,

@gabrielg5
Copy link
Collaborator

Hey @p0dalirius,

great! just one last ask before merging this PR... Can you extract the spnExists resolution to an external function to avoid duplicating those lines?

thank you!

@p0dalirius
Copy link
Contributor Author

Done :)

@gabrielg5 gabrielg5 removed the waiting for response Further information is needed from people who opened the issue or pull request label May 23, 2024
@gabrielg5
Copy link
Collaborator

Merging now..
Thank you!

@gabrielg5 gabrielg5 merged commit 15eff88 into fortra:master May 23, 2024
9 checks passed
@p0dalirius p0dalirius deleted the add-spn-finddelegations.py branch June 4, 2024 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in review This issue or pull request is being analyzed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants