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

Fix exploit token replacement when having multiple matches #167

Merged
merged 3 commits into from
Oct 3, 2021

Conversation

vanjo9800
Copy link
Contributor

While testing my changes on !162, I noticed that there was a problem in the Exploit reporting when there were multiple matches in the file:

➜  pyWhat git:(fix_here_replacement) pywhat fixtures/file | grep -A 2 -B 1 'Twilio Account SID'
Matched on: ACAQDrnjkGtf3iA46rtwsvRiscvMTCw30l
Name: Twilio Account SID
Exploit: Use the command below to verify that the SID is valid:
  $ curl -X GET 'https://api.twilio.com/2010-04-01/Accounts.json' -u ACAQDrnjkGtf3iA46rtwsvRiscvMTCw30l:[AUTH_TOKEN]
--
--
Matched on: ACAWIQTrhbtfozp14V6UTmPyMVUMT0fjjg
Name: Twilio Account SID
Exploit: Use the command below to verify that the SID is valid:
  $ curl -X GET 'https://api.twilio.com/2010-04-01/Accounts.json' -u ACAQDrnjkGtf3iA46rtwsvRiscvMTCw30l:[AUTH_TOKEN]
--
--
Matched on: ACgkQ8jFVDE9H447pKwD6A5xwUqIDprBzr
Name: Twilio Account SID
Exploit: Use the command below to verify that the SID is valid:
  $ curl -X GET 'https://api.twilio.com/2010-04-01/Accounts.json' -u ACAQDrnjkGtf3iA46rtwsvRiscvMTCw30l:[AUTH_TOKEN]

Looking at the code, I found the issue was caused by https://github.com/bee-san/pyWhat/blob/main/pywhat/regex_identifier.py#L31 where a shallow copy was used and the regex match reference was directly manipulated due to the same name. I have change the copy to deep and rename the matched expression data.

After fixing the issue, each token is replaced correspondingly:

➜  pyWhat git:(fix_here_replacement) pywhat fixtures/file | grep -A 2 -B 1 'Twilio Account SID'
Matched on: ACAQDrnjkGtf3iA46rtwsvRiscvMTCw30l
Name: Twilio Account SID
Exploit: Use the command below to verify that the SID is valid:
  $ curl -X GET 'https://api.twilio.com/2010-04-01/Accounts.json' -u ACAQDrnjkGtf3iA46rtwsvRiscvMTCw30l:[AUTH_TOKEN]
--
--
Matched on: ACAWIQTrhbtfozp14V6UTmPyMVUMT0fjjg
Name: Twilio Account SID
Exploit: Use the command below to verify that the SID is valid:
  $ curl -X GET 'https://api.twilio.com/2010-04-01/Accounts.json' -u ACAWIQTrhbtfozp14V6UTmPyMVUMT0fjjg:[AUTH_TOKEN]
--
--
Matched on: ACgkQ8jFVDE9H447pKwD6A5xwUqIDprBzr
Name: Twilio Account SID
Exploit: Use the command below to verify that the SID is valid:
  $ curl -X GET 'https://api.twilio.com/2010-04-01/Accounts.json' -u ACgkQ8jFVDE9H447pKwD6A5xwUqIDprBzr:[AUTH_TOKEN]

@vanjo9800 vanjo9800 marked this pull request as draft October 3, 2021 17:45
@vanjo9800 vanjo9800 marked this pull request as ready for review October 3, 2021 17:51
@bee-san bee-san self-requested a review October 3, 2021 18:57
@bee-san
Copy link
Owner

bee-san commented Oct 3, 2021

This is actually a very cool bug, great description of it and a nice fix 👏🏻 Thank you so much! 👏🏻 👏🏻 👏🏻

@vanjo9800
Copy link
Contributor Author

I will fix the tests now.

@bee-san bee-san merged commit fd14e75 into bee-san:main Oct 3, 2021
P403n1x87 added a commit to P403n1x87/pyWhat that referenced this pull request Oct 20, 2021
It looks like bee-san#167 has introduced a performance regression
by adding what seems to be an unnecessary deep copy. A
shallow dictionary copy seems to work just fine.
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.

2 participants