From: Fabian Keil Date: Tue, 30 Jan 2007 13:05:26 +0000 (+0000) Subject: - Let server_set_cookie() check the expiration date X-Git-Tag: v_3_0_7~353 X-Git-Url: http://www.privoxy.org/gitweb/%40user-manual%40%40actions-help-prefix%40CRUNCH-INCOMING-COOKIES?a=commitdiff_plain;h=bfe821794c2fd5704c7ea7acd735b6581d95047e;p=privoxy.git - Let server_set_cookie() check the expiration date of cookies and don't touch the ones that are already expired. Fixes problems with low quality web applications as described in BR 932612. - Adjust comment in client_max_forwards to reality; remove invalid Max-Forwards headers. --- diff --git a/AUTHORS b/AUTHORS index cc4d8610..df535f1a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -40,6 +40,7 @@ alphabetical order): Ken Arromdee Devin Bayer + Gergely Bor Reiner Buehl Andrew J. Caines Clifford Caoile diff --git a/parsers.c b/parsers.c index fdc2f79c..eb82e481 100644 --- a/parsers.c +++ b/parsers.c @@ -1,4 +1,4 @@ -const char parsers_rcs[] = "$Id: parsers.c,v 1.84 2007/01/24 12:56:52 fabiankeil Exp $"; +const char parsers_rcs[] = "$Id: parsers.c,v 1.85 2007/01/26 15:33:46 fabiankeil Exp $"; /********************************************************************* * * File : $Source: /cvsroot/ijbswa/current/parsers.c,v $ @@ -45,6 +45,11 @@ const char parsers_rcs[] = "$Id: parsers.c,v 1.84 2007/01/24 12:56:52 fabiankeil * * Revisions : * $Log: parsers.c,v $ + * Revision 1.85 2007/01/26 15:33:46 fabiankeil + * Stop filter_header() from unintentionally removing + * empty header lines that were enlisted by the continue + * hack. + * * Revision 1.84 2007/01/24 12:56:52 fabiankeil * - Repeat the request URL before logging any headers. * Makes reading the log easier in case of simultaneous requests. @@ -2646,19 +2651,29 @@ jb_err client_max_forwards(struct client_state *csp, char **header) { if (1 == sscanf(*header, "Max-Forwards: %u", &max_forwards)) { - if (max_forwards-- > 0) + if (max_forwards > 0) { - snprintf(*header, strlen(*header)+1, "Max-Forwards: %u", max_forwards); + snprintf(*header, strlen(*header)+1, "Max-Forwards: %u", --max_forwards); log_error(LOG_LEVEL_HEADER, "Max-Forwards header for %s request replaced with: %s", csp->http->gpc, *header); } else { - /* FIXME: Follow spec and intercept the request. */ + /* + * direct_response() which was called earlier + * in chat() should prevent that we ever get + * here. + */ log_error(LOG_LEVEL_ERROR, "Non-intercepted %s request with Max-Forwards zero!", csp->http->gpc); + assert(max_forwards != 0); } } + else + { + log_error(LOG_LEVEL_ERROR, "Crunching invalid header: %s", *header); + freez(*header); + } } return JB_ERR_OK; @@ -3222,8 +3237,13 @@ jb_err server_http(struct client_state *csp, char **header) * Function : server_set_cookie * * Description : Handle the server "cookie" header properly. - * Log cookie to the jar file. Then "crunch" it, - * or accept it. Called from `sed'. + * Log cookie to the jar file. Then "crunch", + * accept or rewrite it to a session cookie. + * Called from `sed'. + * + * TODO: Allow the user to specify a new expiration + * time to cause the cookie to expire even before the + * browser is closed. * * Parameters : * 1 : csp = Current client state (buffers, headers, etc...) @@ -3238,6 +3258,12 @@ jb_err server_http(struct client_state *csp, char **header) *********************************************************************/ jb_err server_set_cookie(struct client_state *csp, char **header) { + time_t now; + time_t cookie_time; + struct tm tm_now; + struct tm tm_cookie; + time(&now); + #ifdef FEATURE_COOKIE_JAR if (csp->config->jar) { @@ -3248,9 +3274,7 @@ jb_err server_set_cookie(struct client_state *csp, char **header) * the %z field in strftime() */ char tempbuf[ BUFFER_SIZE ]; - time_t now; - struct tm tm_now; - time (&now); + #ifdef HAVE_LOCALTIME_R tm_now = *localtime_r(&now, &tm_now); #elif FEATURE_PTHREAD @@ -3311,22 +3335,122 @@ jb_err server_set_cookie(struct client_state *csp, char **header) next_tag = cur_tag + strlen(cur_tag); } - /* Is this the "Expires" tag? */ + /* + * Check the expiration date to see + * if the cookie is still valid, if yes, + * rewrite it to a session cookie. + */ if (strncmpic(cur_tag, "expires=", 8) == 0) { - /* Delete the tag by copying the rest of the string over it. - * (Note that we cannot just use "strcpy(cur_tag, next_tag)", - * since the behaviour of strcpy is undefined for overlapping - * strings.) + char *match; + /* + * Try the valid time formats we know about. + * + * XXX: Maybe the log messages should be removed + * for the next stable release. They just exist to + * see which time format gets the most hits and + * should be checked for first. */ - memmove(cur_tag, next_tag, strlen(next_tag) + 1); + if (NULL != (match = strptime(cur_tag, "expires=%a, %e-%b-%y %H:%M:%S ", &tm_cookie))) + { + log_error(LOG_LEVEL_HEADER, + "cookie \'%s\' send by %s appears to be using time format 1.", + csp->http->url, *header); + } + else if (NULL != (match = strptime(cur_tag, "expires=%A, %e-%b-%Y %H:%M:%S ", &tm_cookie))) + { + log_error(LOG_LEVEL_HEADER, + "cookie \'%s\' send by %s appears to be using time format 2.", + csp->http->url, *header); - /* That changed the header, need to issue a log message */ - changed = 1; + } + else if (NULL != (match = strptime(cur_tag, "expires=%a, %e-%b-%Y %H:%M:%S ", &tm_cookie))) + { + log_error(LOG_LEVEL_HEADER, + "cookie \'%s\' send by %s appears to be using time format 3.", + csp->http->url, *header); + } + + /* Did any of them match? */ + if (NULL == match) + { + /* + * Nope, treat it as if it was still valid. + * + * XXX: Should we remove the whole cookie instead? + */ + log_error(LOG_LEVEL_ERROR, + "Can't parse %s. Unsupported time format?", cur_tag); + memmove(cur_tag, next_tag, strlen(next_tag) + 1); + changed = 1; + } + else + { + /* + * Yes. Check if the cookie is still valid. + * + * If the cookie is already expired it's probably + * a delete cookie and even if it isn't, the browser + * will discard it anyway. + */ + + /* + * XXX: timegm() isn't available on some AmigaOS + * versions and our replacement doesn't work. + * + * Our options are to either: + * + * - disable session-cookies-only completely if timegm + * is missing, + * + * - to simply remove all expired tags, like it has + * been done until Privoxy 3.0.6 and to live with + * the consequence that it can cause login/logout + * problems on servers that don't validate their + * input properly, or + * + * - to replace it with mktime in which + * case there is a slight chance of valid cookies + * passing as already expired. + * + * This is the way it's currently done and it's not + * as bad as it sounds. If the missing GMT offset is + * enough to change the result of the expiration check + * the cookie will be only valid for a few hours + * anyway, which in many cases will be shorter + * than a browser session. + */ + cookie_time = timegm(&tm_cookie); + if (cookie_time - now < 0) + { + log_error(LOG_LEVEL_HEADER, + "Cookie \'%s\' is already expired and can pass unmodified.", *header); + /* Just in case some clown sets more then one expiration date */ + cur_tag = next_tag; + } + else + { + log_error(LOG_LEVEL_HEADER, + "Cookie \'%s\' is still valid and has to be rewritten.", *header); + + /* + * Delete the tag by copying the rest of the string over it. + * (Note that we cannot just use "strcpy(cur_tag, next_tag)", + * since the behaviour of strcpy is undefined for overlapping + * strings.) + */ + memmove(cur_tag, next_tag, strlen(next_tag) + 1); + + /* That changed the header, need to issue a log message */ + changed = 1; + + /* + * Note that the next tag has now been moved to *cur_tag, + * so we do not need to update the cur_tag pointer. + */ + } + } - /* Note that the next tag has now been moved to *cur_tag, - * so we do not need to update the cur_tag pointer. - */ } else { @@ -3337,7 +3461,9 @@ jb_err server_set_cookie(struct client_state *csp, char **header) if (changed) { - log_error(LOG_LEVEL_HEADER, "Changed cookie to a temporary one."); + assert(NULL != *header); + log_error(LOG_LEVEL_HEADER, "Cookie rewritten to a temporary one: %s", + *header); } }