Skip to content

Conversation

@rhowinespinoza
Copy link

Fixes:

  1. Fix incorrect end tag
  2. Fix infinite request loop
  3. Use SERVER_NAME instead of HTTP_HOST to prevent SSRF attack

Rhowin Espinoza added 3 commits October 15, 2020 19:39
…T is controllable by the user and isn’t required to match the requested server. The attacker only needs to set the HTTP Host header to cause the server to attempt a GET request connect to any server, including internal networks.
An HTTP GET request to /js/whichbrowser/?a will cause the server to continuously send HTTP GET requests to itself because the detect url will be /js/whichbrowser/?adetect.php.
@kornicameister
Copy link

kornicameister commented Apr 8, 2022

It's been 2 years already so asking if there's a plan to get this merged is rather awkward.
I will though ask @rhowinespinoza ; are you able to point out what needed to happen for 2nd point to occur?

@rhowinespinoza
Copy link
Author

It's been 2 years already so asking if there's a plan to get this merged is rather awkward. I will though ask @rhowinespinoza ; are you able to point out what needed to happen for 2nd point to occur?

@kornicameister An HTTP GET request to /js/whichbrowser/?a will cause the server to continuously send HTTP GET requests to itself because the detect url will be /js/whichbrowser/?adetect.php.

@kornicameister
Copy link

image

I know... :D

@kornicameister
Copy link

@rhowinespinoza were you able to battle test your solution? Is it a script, test somewhere that you used to find and confirm the fix? I mean, I would like to have it used to verify things on my end but given I am not exactly on friendly terms with PHP (although maybe I don't need to), any help would be appreciated.

@NielsLeenheer NielsLeenheer merged commit b7b2bb8 into WhichBrowser:master Apr 12, 2022
@kornicameister
Copy link

oh, lovely :* thanks @NielsLeenheer

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