benlucchesi / grails-cookie-session

cookie sessions for grails applications
28 stars 32 forks source link

Malformed cookie data can cause RuntimeException #27

Closed jrh3k5 closed 10 years ago

jrh3k5 commented 10 years ago

I'm seeing these errors pop up multiple times in our logs:

2013-11-18 14:15:58,027 [http-bio-8082-exec-10] ERROR cookiesession.CookieSessionRepository - cause: null
java.lang.RuntimeException: = character not at end of base64 value
at org.codehaus.groovy.runtime.EncodingGroovyMethods.decodeBase64(EncodingGroovyMethods.java:160)
at org.codehaus.groovy.runtime.dgm$644.invoke(Unknown Source)
at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite$PojoMetaMethodSiteNoUnwrapNoCoerce.invoke(PojoMetaMethodSite.java:271)
at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite.call(PojoMetaMethodSite.java:53)
at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:112)
at com.granicus.grails.plugins.cookiesession.CookieSessionRepository.deserializeSession(CookieSessionRepository.groovy:330)
at com.granicus.grails.plugins.cookiesession.CookieSessionRepository$deserializeSession$8.callCurrent(Unknown Source)
at com.granicus.grails.plugins.cookiesession.CookieSessionRepository.restoreSession(CookieSessionRepository.groovy:242)
at com.granicus.grails.plugins.cookiesession.SessionRepositoryRequestWrapper.restoreSession(SessionRepositoryRequestWrapper.java:58)
at com.granicus.grails.plugins.cookiesession.CookieSessionFilter.doFilterInternal(CookieSessionFilter.java:74)

The fact that someone's sending in cookies with invalid values (or no cookies at all) isn't a problem, but these messages kind of spam our logs and obfuscate real errors.

benlucchesi commented 10 years ago

From what I've noticed, this occurs when the cookie specification is changed between requests. When a cookie is sent to the client it has various parameters set: name, path, secure, domain set, maxAge, etc. When the browser sends a request to the server, it sends only the cookie name and value, and cookies with duplicate names are allowed. If you change the domain or path of a cookie then the browser will store multiple cookies with the same name instead of replacing a cookie. Then when the browser sends a request, it will send multiple cookies with the same name back to the server. And because only the name and value are sent in the request, the server tries to use all of them that match the name it set. In the process of reconstructing a base64 string that stores the session, it mashes up a bunch of string and fails to deserialize.

I think this situation could also occur if you have multiple applications running under the same domain that both use the cookie-session plugin, because both apps will be storing cookies with common names, but I haven't tested for that.

What really sucks is that this situation is unrecoverable because the only way you can cause the browser to delete a cookie is if you construct it with the same parameters (name, path, etc) and set maxAge to 0. And because the server can't determine which is the offending cookie and what parameters are set on it (in the browser), the server can't cause the browser to delete it.

If you are running multiple applications under the same domain, or if you need to recover from this situation, change the cookie name in the config so that its unique to the application and don't mess with the other parameters once the application is started and cookies are being stored in browsers. And make sure you're using a reasonable session timeout so that you don't have to ask users to clear cookies to get your site to work.

Also, I've been working on an update for another user that uses Servlet 3.0 functionality to configure the cookie's parameters using the ServletContext.SessionCookieConfig as well expand the number of cookie properties you can configure.

Ben Lucchesi | Chief Software Architect | Granicus Inc. 600 Harrison Street, Suite 120 San Francisco, CA 94107 work: 415.357.3618 x1300 | fax: 415.618.0102 | cell: 775.250.3396


From: jrh3k5 [notifications@github.com] Sent: Wednesday, November 20, 2013 1:50 PM To: benlucchesi/grails-cookie-session-v2 Subject: [grails-cookie-session-v2] Malformed cookie data can cause RuntimeException (#27)

I'm seeing these errors pop up multiple times in our logs:

2013-11-18 14:15:58,027 [http-bio-8082-exec-10] ERROR cookiesession.CookieSessionRepository - cause: null java.lang.RuntimeException: = character not at end of base64 value at org.codehaus.groovy.runtime.EncodingGroovyMethods.decodeBase64(EncodingGroovyMethods.java:160) at org.codehaus.groovy.runtime.dgm$644.invoke(Unknown Source) at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite$PojoMetaMethodSiteNoUnwrapNoCoerce.invoke(PojoMetaMethodSite.java:271) at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite.call(PojoMetaMethodSite.java:53) at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:112) at com.granicus.grails.plugins.cookiesession.CookieSessionRepository.deserializeSession(CookieSessionRepository.groovy:330) at com.granicus.grails.plugins.cookiesession.CookieSessionRepository$deserializeSession$8.callCurrent(Unknown Source) at com.granicus.grails.plugins.cookiesession.CookieSessionRepository.restoreSession(CookieSessionRepository.groovy:242) at com.granicus.grails.plugins.cookiesession.SessionRepositoryRequestWrapper.restoreSession(SessionRepositoryRequestWrapper.java:58) at com.granicus.grails.plugins.cookiesession.CookieSessionFilter.doFilterInternal(CookieSessionFilter.java:74)

The fact that someone's sending in cookies with invalid values (or no cookies at all) isn't a problem, but these messages kind of spam our logs and obfuscate real errors.

— Reply to this email directly or view it on GitHubhttps://github.com/benlucchesi/grails-cookie-session-v2/issues/27.

jrh3k5 commented 10 years ago

That definitely makes sense, especially since I changed the path of the cookies from "/" to the path of my application.

The problem, though, is that we're still vulnerable to log spam if somebody just sends requests to our application with garbage cookie data. I've got an upstream filter that now validates that the session repository can correctly assemble the session (and deletes the cookies and filters them out of the request when it can't), but this is obviously undesirable because I'm now assembling the session twice for each request).

Would a try/catch block around the assembly of the data from a String into the byte[] data be doable?

benlucchesi commented 10 years ago

Totally understandable....

So there is a try-catch around the code that assembles the string, and when it fails, you get the log message you're seeing. I don't want to bury this message because this scenario shouldn't be happening and your users are failing to get their session. The only reason you're seeing these messages is because of the change you made to the domain in the cookie, so its a good thing that we know about it.

As a suggestion, if you don't want to see these messages, you can turn off logging for the CookieSessionRepository:

log4j = { off 'com.granicus.grails.plugins.cookiesession.CookieSessionRepository' }

Do you have any other suggestions on how to track these types of exceptions?

Ben Lucchesi | Chief Software Architect | Granicus Inc. 600 Harrison Street, Suite 120 San Francisco, CA 94107 work: 415.357.3618 x1300 | fax: 415.618.0102 | cell: 775.250.3396


From: jrh3k5 [notifications@github.com] Sent: Thursday, November 21, 2013 6:33 AM To: benlucchesi/grails-cookie-session-v2 Cc: Benjamin Lucchesi Subject: Re: [grails-cookie-session-v2] Malformed cookie data can cause RuntimeException (#27)

That definitely makes sense, especially since I changed the path of the cookies from "/" to the path of my application.

The problem, though, is that we're still vulnerable to log spam if somebody just sends requests to our application with garbage cookie data. I've got an upstream filter that now validates that the session repository can correctly assemble the session (and deletes the cookies and filters them out of the request when it can't), but this is obviously undesirable because I'm now assembling the session twice for each request).

Would a try/catch block around the assembly of the data from a String into the byte[] data be doable?

— Reply to this email directly or view it on GitHubhttps://github.com/benlucchesi/grails-cookie-session-v2/issues/27#issuecomment-28988595.

benlucchesi commented 10 years ago

I'm going to close this issue out for the time being. If you have a suggestion on how to better handle the scenario as described above, please reopen this issue.

thanks!