[interchange] Fix "HTTP Response Splitting" security exploit

David Christensen <[email protected]>
Newsgroups gmane.comp.web.interchange.cvs
Message-ID <[email protected]>
commit c2d7cc435b71ffaaa1e6e1050566a087f8b5e510
Author: David Christensen <[email protected]>
Date:   Mon Mar 22 17:29:22 2010 -0500

    Fix "HTTP Response Splitting" security exploit
    
    Discovery and patch from Justin Otten <[email protected]>:
    
    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;
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.