Skip to content

Fix exploit token replacement when having multiple matches#167

Merged
bee-san merged 3 commits intobee-san:mainfrom
vanjo9800:fix_here_replacement
Oct 3, 2021
Merged

Fix exploit token replacement when having multiple matches#167
bee-san merged 3 commits intobee-san:mainfrom
vanjo9800:fix_here_replacement

Conversation

@vanjo9800
Copy link
Copy Markdown
Contributor

@vanjo9800 vanjo9800 commented Oct 3, 2021

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 force-pushed the fix_here_replacement branch from 4d6c41c to afcb78e Compare October 3, 2021 17:50
@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
Copy Markdown
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
Copy Markdown
Contributor Author

vanjo9800 commented Oct 3, 2021

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
@P403n1x87
perf: fix RegexIdentifier.check regression
7c657cf 
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.
@P403n1x87 P403n1x87 mentioned this pull request Oct 20, 2021
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bee-san bee-san bee-san approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vanjo9800 @bee-san