[interchange] Fix "HTTP Response Splitting" security exploit

David Christensen interchange-cvs at icdevgroup.org
Thu Mar 25 04:43:34 UTC 2010


commit c2d7cc435b71ffaaa1e6e1050566a087f8b5e510
Author: David Christensen <david at endpoint.com>
Date:   Mon Mar 22 17:29:22 2010 -0500

    Fix "HTTP Response Splitting" security exploit
    
    Discovery and patch from Justin Otten <justin.otten at gmail.com>:
    
    Added new method to Util.pm for scrubbing newlines from header data.
    Updated all discovered instances of the use of the "Location" header
    ran the URL through the routine.

 code/SystemTag/deliver.coretag     |    4 ++++
 lib/Vend/Dispatch.pm               |    1 +
 lib/Vend/Error.pm                  |    2 ++
 lib/Vend/Parse.pm                  |    3 +++
 lib/Vend/Payment/GoogleCheckout.pm |    1 +
 lib/Vend/Payment/Worldpay.pm       |    1 +
 lib/Vend/Util.pm                   |   11 +++++++++++
 7 files changed, 23 insertions(+), 0 deletions(-)
---
diff --git a/code/SystemTag/deliver.coretag b/code/SystemTag/deliver.coretag
index 6f8f006..d6e221b 100644
--- a/code/SystemTag/deliver.coretag
+++ b/code/SystemTag/deliver.coretag
@@ -42,6 +42,10 @@ sub {
 
 	## This is a bounce, returns
 	if($opt->{location}) {
+		$type = Vend::Util::header_data_scrub($type);
+		$opt->{status} = Vend::Util::header_data_scrub($opt->{status});
+		$opt->{location} = Vend::Util::header_data_scrub($opt->{location});
+
 		$type and $Tag->tag( {
 						op => 'header',
 						name => 'Content-Type',
diff --git a/lib/Vend/Dispatch.pm b/lib/Vend/Dispatch.pm
index d0b7d29..cf73c1e 100644
--- a/lib/Vend/Dispatch.pm
+++ b/lib/Vend/Dispatch.pm
@@ -1582,6 +1582,7 @@ EOF
 			grep { !$Vend::Cfg->{BounceReferrals_hide}->{$_} }
 			sort keys %CGI::values;
 		my $url = vendUrl($path eq '' ? $Vend::Cfg->{DirectoryIndex} : $path, undef, undef, { form => $form, match_security => 1 });
+		$url = header_data_scrub($url);
 		my $msg = get_locale_message(
 			301,
 			"Redirected to %s.",
diff --git a/lib/Vend/Error.pm b/lib/Vend/Error.pm
index 79df7a1..3be8bec 100644
--- a/lib/Vend/Error.pm
+++ b/lib/Vend/Error.pm
@@ -56,6 +56,8 @@ sub get_locale_message {
 	}
 	if($message !~ /\s/) {
 		if($message =~ /^http:/) {
+			$message = header_data_scrub($message);
+
 			$Vend::StatusLine =~ s/([^\r\n])$/$1\r\n/;
 			$Vend::StatusLine .= "Status: 302 Moved\r\nLocation: $message\r\n";
 			$message = "Redirected to $message.";
diff --git a/lib/Vend/Parse.pm b/lib/Vend/Parse.pm
index 856d6cf..4d4efa9 100644
--- a/lib/Vend/Parse.pm
+++ b/lib/Vend/Parse.pm
@@ -751,6 +751,9 @@ sub start {
 			if(! $attr->{href} and $attr->{page}) {
 				$attr->{href} = Vend::Interpolate::tag_area($attr->{page});
 			}
+
+			$attr->{href} = header_data_scrub($attr->{href});
+
 			$Vend::StatusLine = '' if ! $Vend::StatusLine;
 			$Vend::StatusLine .= "\n" if $Vend::StatusLine !~ /\n$/;
 			$Vend::StatusLine .= <<EOF if $attr->{target};
diff --git a/lib/Vend/Payment/GoogleCheckout.pm b/lib/Vend/Payment/GoogleCheckout.pm
index 8308119..d186b90 100644
--- a/lib/Vend/Payment/GoogleCheckout.pm
+++ b/lib/Vend/Payment/GoogleCheckout.pm
@@ -757,6 +757,7 @@ use Data::Dumper; # for debugging
 # print Dumper($xmlin); # for debugging
 #print Dumper($::Session);  
   unless (($xmlin->{'error-message'}) or ($diagnose)) {
+	  $redirecturl = Vend::Util::header_data_scrub($redirecturl);
 
 $::Tag->tag({ op => 'header', body => <<EOB });
 Status: 302 moved
diff --git a/lib/Vend/Payment/Worldpay.pm b/lib/Vend/Payment/Worldpay.pm
index 5458e33..f0be487 100644
--- a/lib/Vend/Payment/Worldpay.pm
+++ b/lib/Vend/Payment/Worldpay.pm
@@ -474,6 +474,7 @@ else{
 	 $redirecturl .= "&postcode=$postcode&country=$country&email=$email&tel=$tel&MC_mv_order_number=$cartId&MC_callback=$callbackurl&MC_affsubtotal=$affsubtotal";
 	
 	 $redirecturl .= "&fixContact" if ($fixcontact == 1);
+	 $redirecturl = Vend::Util::header_data_scrub($redirecturl);
 
 ::logDebug("WP:".__LINE__.": URL = $redirecturl");
  
diff --git a/lib/Vend/Util.pm b/lib/Vend/Util.pm
index d958e50..646a119 100644
--- a/lib/Vend/Util.pm
+++ b/lib/Vend/Util.pm
@@ -50,6 +50,7 @@ unless( $ENV{MINIVEND_DISABLE_UTF8} ) {
 	generate_key
 	get_option_hash
 	hash_string
+	header_data_scrub
 	hexify
 	is_hash
 	is_no
@@ -2484,6 +2485,16 @@ sub backtrace {
     undef;
 }
 
+sub header_data_scrub {
+	my ($head_data) = @_;
+
+	## "HTTP Response Splitting" Exploit Fix
+	## http://www.securiteam.com/securityreviews/5WP0E2KFGK.html
+	$head_data =~ s/(?:%0[da]|[\r\n]+)+//ig;
+
+	return $head_data;
+}
+
 ### Provide stubs for former Vend::Util functions relocated to Vend::File
 *canonpath = \&Vend::File::canonpath;
 *catdir = \&Vend::File::catdir;



More information about the interchange-cvs mailing list