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

[8.x] Throw if tag is passed but is not supported #41479

Merged
merged 3 commits into from Mar 14, 2022

Conversation

tm1000
Copy link
Contributor

@tm1000 tm1000 commented Mar 14, 2022

Original PR for 9.x: #41469

This allows Laravel to throw a DecryptException (which is ignored by bug catching libraries like Sentry) instead of throwing an open_ssl exception of openssl_decrypt(): The tag cannot be used because the cipher algorithm does not support AEAD when a tag is passed with a value but AEAD is not supported.

This commonly seems to happen in malicious web scripts that modify the encrypted cookie values to insert exploitable tag values. The way Laravel works tag is never exploitable in this way. However it ends up causing a 500 error and additionally if you are using any error reporting engine you end up with thousands of issues a day with openssl_decrypt(): The tag cannot be used because the cipher algorithm does not support AEAD tag.

One such example of a compromised cookie:

{
iv: OD2GF9/yd3m6Sn6cfC2ZHw==, 
mac: e226b1d78883cde51b27317d4b3a99605c07be1fdc61f303180fad07854dd1f7, 
tag: -1 OR 2+406-406-1=0+0+0+1 -- , 
value: GA8xQCz5RCR2PvcK+HtcBbUYM9zxA62Tcz9M/5F2Eh6BzGrpqkIsNbne5Hw/ETu2mFvFE/Tk8Zmqj6wB45Ldty9CIZgSXtFRsiv/elTQlaB2Hmf9UzuKC9w1y/xwlXrw
}

An easy way to exploit this yourself is to go to any Laravel 9 site. Copy out the base64 session cookie, modify the tag key to add any value and then re-encode the cookie. As long as the site does not support AEAD it will cause a 500 error.

This simply throws a DecryptException if AEAD is not supported and a tag was still sent. Throwing DecryptException allows Laravel to function without 500ing and prevents bug catchers from reporting this.

@taylorotwell taylorotwell merged commit a556263 into laravel:8.x Mar 14, 2022
@driesvints
Copy link
Member

Thanks @tm1000

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.

None yet

3 participants