[interchange] Fix a bug in read_cookie's code path when using the single-arg form

David Christensen interchange-cvs at icdevgroup.org
Wed Mar 9 07:25:56 UTC 2011


commit d32ba730f652480ed506d5cd0409e1f72881e330
Author: David Christensen <david at endpoint.com>
Date:   Wed Mar 9 01:25:31 2011 -0600

    Fix a bug in read_cookie's code path when using the single-arg form
    
    This issue was caused by a bug in the interchange read_cookie codepath
    which was being too lentient about its parsing of $CGI::cookie when
    looking up a specific cookie's value.
    
    Certain $CGI::cookie strings and requested cookie names can result in
    returning the wrong value for the cookie given the following
    circumstances: $CGI::cookie contains a value portion of the keyvalue
    pairs which include a word-break character, the (case-insensitive)
    target name and then an equals sign.  Additionally, this matching
    substring would need to appear before the actual cookie for the key in
    question.
    
    Example:
    
    Given $ENV{HTTP_COOKIE}:
      'foo.tracker={"url":"http://www.site.com/?mv_source=blah","count":3}; MV_SOURCE=foo'
    
    [read-cookie] without arguments would correctly parse and return the
    expected keypairs, however [read-cookie MV_SOURCE] would scan the
    $CGI::cookie string for a word-break, the specific cookie name, a
    literal '=' and then proceed to return the literal:
    
      MV_SOURCE => 'blah","count":3}'
    
    This fix tightens up the parsing to only look at the start of the
    string or immediately after a ';' (with optional whitespace between)
    when parsing a specific cookie value.
    
    ---
    Some additional comments:
    
    I had difficulty locating a specification for the cookie keys/values
    themselves, but I wonder if we should remove the /i regex modifier, as
    I'd personally expect cookie names to be case-sensitive.  Left in for
    backwards-compatibility.
    
    Additionally, the setter of the aforementioned cookie should likely
    have used some form of uriencoding instead of having the raw '{}='
    characters, however that's no excuse for us to barf on bad behavior.

 lib/Vend/Util.pm |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
---
diff --git a/lib/Vend/Util.pm b/lib/Vend/Util.pm
index d047c34..e152786 100644
--- a/lib/Vend/Util.pm
+++ b/lib/Vend/Util.pm
@@ -2006,7 +2006,7 @@ sub read_cookie {
 	my ($lookfor, $string) = @_;
 	$string = $CGI::cookie
 		unless defined $string;
-	return undef unless $string =~ /\b$lookfor=([^\s;]+)/i;
+    return undef unless $string =~ /(?:^|;)\s*\Q$lookfor\E=([^\s;]+)/i;
  	return unescape_chars($1);
 }
 



More information about the interchange-cvs mailing list