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

Revert #190 to enable null-auth without explicit specification #208

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

NeffIsBack
Copy link
Contributor

PR to test if #207 is fixed with empty string auth (null auth)

@NeffIsBack NeffIsBack added the bug-fix This Pull Request fixes a bug label Mar 14, 2024
@NeffIsBack NeffIsBack marked this pull request as ready for review March 15, 2024 23:29
@NeffIsBack NeffIsBack removed the bug-fix This Pull Request fixes a bug label Mar 15, 2024
@NeffIsBack NeffIsBack changed the title Revert #190 to enable functionality without explicit auth Revert #190 to enable null-auth without explicit specification Mar 15, 2024
@mpgn
Copy link
Collaborator

mpgn commented Mar 16, 2024

to be tested but it should work ok

Fix this issue

image-29.png

@NeffIsBack
Copy link
Contributor Author

Hmm that will always show the null auth tho. A lot of unnecessary output when you do nxc smb <ip>/24 --shares.
I would vote for just leaving it as it is and wait until that error occurs the next time. Then i can debug it properly, i really don't know how this should have happened (refering to the index out of bounds)

@mpgn
Copy link
Collaborator

mpgn commented Mar 17, 2024

Show me a screenshot of the output you see because I don't understand what you mean

@mpgn
Copy link
Collaborator

mpgn commented Mar 17, 2024

If you don't want any output if no -u -p then this pr should not be merged

@NeffIsBack
Copy link
Contributor Author

image
That one is what i meant. That was the error you are aiming to fix right?

@NeffIsBack
Copy link
Contributor Author

If you don't want any output if no -u -p then this pr should not be merged

Yeah i just would like to roll it back to how it was all that time.

@mpgn
Copy link
Collaborator

mpgn commented Mar 18, 2024

Then merge the pr 🙂

@mpgn
Copy link
Collaborator

mpgn commented Mar 18, 2024

to be tested but it should work ok

Fix this issue

image-29.png

Before and after

308350245-0e54cb89-de5b-4a50-a7d6-4394c37e695e.png

Follow the code with an empty array (the fix don't allow empty array) you will see it makes no sens to send an empty array since login will be false but username and password to "" will bypass the login check.
Then you go on shares function without a login working making the exception.

With the fix, you send "" to the smb login function and not empty array making it properly check the login

To simplify, my commit is the equivalent to send -u "" -p "" but without adding them into the command line

@mpgn
Copy link
Collaborator

mpgn commented Mar 18, 2024

On the other end, i didn't check the gen-relay option, if not working then my commit need to be removed

Can you check ? 🙏

@Marshall-Hallenbeck
Copy link
Collaborator

Just tested, gen relay works on this PR

@NeffIsBack NeffIsBack merged commit 6b6decd into main Mar 19, 2024
2 checks passed
@NeffIsBack NeffIsBack deleted the neff-fix-207 branch March 19, 2024 23:58
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