spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.55k stars 5.79k forks source link

Base64 Padding Issue in AbstractRememberMeServices.decodeCookie #15127

Closed guqing closed 1 month ago

guqing commented 1 month ago

Describe the bug The decodeCookie method in AbstractRememberMeServices attempts to pad the cookie value for Base64 decoding. However, the current implementation may not correctly calculate the padding needed to ensure the string length is a multiple of 4, which is a requirement for Base64 decoding.

Current Implementation

for (int j = 0; j < cookieValue.length() % 4; j++) {
    cookieValue = cookieValue + "=";
}

To Reproduce Steps to reproduce the behavior.

  1. Pass a cookie value whose length modulo 4 is not 0 to the decodeCookie method without the correct padding.
  2. Observe the IllegalArgumentException due to invalid Base64 string format.

Expected behavior The method should add padding characters so that the length of cookieValue becomes a multiple of 4 to adhere to Base64 decoding requirements. The padding should be calculated as 4 - (cookieValue.length() % 4) and should only add padding if the result is less than 4.

Impact Without this fix, the decodeCookie method may throw IllegalArgumentException when attempting to decode improperly padded Base64 strings, leading to unhandled exceptions and potential disruptions in the remember-me authentication flow.

Sample The 123 bit base64 encoding here

YWRtaW46MTcxODk2NDE3NDgwODpTSEEtMjU2OmNkOTM0ZTAyZWQ4NGJmMzc1ZTA4MmE1OWU4YTA3NTNiMzA3ODg1MjZmYzA3YjgyYzVmY2Y3YmJiYzdjYzRkNWU

will become the following code after passing through that section of code:

YWRtaW46MTcxODk2NDE3NDgwODpTSEEtMjU2OmNkOTM0ZTAyZWQ4NGJmMzc1ZTA4MmE1OWU4YTA3NTNiMzA3ODg1MjZmYzA3YjgyYzVmY2Y3YmJiYzdjYzRkNWU===

but the expected result should be

YWRtaW46MTcxODk2NDE3NDgwODpTSEEtMjU2OmNkOTM0ZTAyZWQ4NGJmMzc1ZTA4MmE1OWU4YTA3NTNiMzA3ODg1MjZmYzA3YjgyYzVmY2Y3YmJiYzdjYzRkNWU=

If it is confirmed that this is a problem, I am willing to try to solve it

guqing commented 1 month ago

Sorry, I misunderstood. I will close this issue