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 Type Juggling Bypass in Auth Functions #2542

Merged
merged 1 commit into from Aug 7, 2019
Merged

Conversation

Wocanilo
Copy link
Contributor

@Wocanilo Wocanilo commented Aug 2, 2019

The auth functions are vulnerable to Type Jugging attacks. Non strict comparisions can be used by an attacker to bypass auth and gain access to the admin page and API.

If PHP decides that both operands looks like numbers, it will convert both an perform numeric comparision. Ex: "0e348324" == "0e99" is true

This commit fixes 4 vulnerabilities:

  • If an user password is saved as plaintext and looks like a number, for example, "0e23", an attacker can bypass login by submitting an input that looks like a number, "0e99".
  • If an user cookie looks like a number, for example "0e210903587487700652447287594086", an attacker can bypass login by sending a cookie that also looks like a number, "0e32".
  • If an user signature looks like a number, for example, "0e344332", an attacker can bypass API auth by sending a signature that also looks like a number, "0e84"

At last, the time limited token functionality can be abused to gain unauthorised access to the API without any requisite. Again, a type juggling attack is posible, but this time the attacker controls the result of the MD5, so thousands of requests can be made till the resultand hash takes the form of a number. We can bypass the YOURLS_NONCE_LIFE by introducing decimals in the timestamp, making the attack viable.

References:
https://www.owasp.org/images/6/6b/PHPMagicTricks-TypeJuggling.pdf

Copy link

@aramburu aramburu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add to master ASAP
@dgw

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are probably more places where === should be used instead of ==, that aren't security-related, too. This is a good idea.

@dgw dgw added the security label Aug 6, 2019
@LeoColomb LeoColomb merged commit 9e36c67 into YOURLS:master Aug 7, 2019
@skaag
Copy link

skaag commented Jan 7, 2020

I have a test yourls installation that some bots were able to find and are once in a while injecting some malicious shortened URL into... I applied this patch, but it's still happening. Any ideas?

@ozh
Copy link
Member

ozh commented Jan 8, 2020

@skaag most likely something else involved. URL of your test install ?

@skaag
Copy link

skaag commented Jan 8, 2020

@skaag most likely something else involved. URL of your test install ?

This is it (It has just 3 shortened links on it as a test, works fine so far):
https://gns.to/

@skaag
Copy link

skaag commented Jan 8, 2020

@ozh :
Steps I took today: I changed the cookie, and the password for the only user in that system (and that user is not admin).

@ozh
Copy link
Member

ozh commented Jan 8, 2020

Soooooo.... "bots" found and are "injecting" URL ... on your publicly available interface? Hmmkey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants