[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;