search

intrigueio / intrigue-ident

Application and Service Fingerprinting
https://core.intrigue.io
Other
131 stars 40 forks source link

false positive on open redirect check #4

Closed digininja closed 4 years ago

digininja commented 5 years ago

Your open redirect check has this regex in it:

/\?url=http/i

But that is used by quite a few other things, on my site, most of the social media links match it:

<a title="Share on Twitter" id="twitter-share" href="https://twitter.com/share?text=DigiNinja&amp;url=https://digi.ninja/index.php" rel="noopener" target="_blank"><i class="fa fa-twitter"></i></a>

I think it is too vague a check to determine open redirect based off that. It also took me a while to track down why you thought there was an open redirect, maybe giving the reason for the decision would be good as it would help determine whether it is a false positive or not.

jcran commented 5 years ago

Awesome, thanks for ticketing. Agree, way too loose. It's been flagging on the oembed wp plugin too. Let me know if you have thoughts on specific items to tighten it up.

digininja commented 5 years ago

Maybe change the finding to "potential" and then log the suspicious URL.

That might but work with the way your implementation though.

On Thu, 18 Apr 2019, 17:48 Jonathan Cran, notifications@github.com wrote:

Awesome, thanks for ticketing. Agree, way too loose. It's been flagging on the oembed wp plugin too. Let me know if you have thoughts on specific items to tighten it up.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/intrigueio/intrigue-ident/issues/4#issuecomment-484588667, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA4SWPPCQBKLPYFDQ3E5CLPRCQ4DANCNFSM4HG5DW7Q .

jcran commented 4 years ago

Functionality is deprecated, marking this as closed. Thanks again for the report @digininja