From 22dde02776c67f9b98b4c96d985519a6021dbc52 Mon Sep 17 00:00:00 2001 From: Paul Driver Date: Wed, 22 Jul 2009 21:22:25 +0000 Subject: [PATCH] Improved security for PayPalStd --- lib/WebGUI/Shop/PayDriver/PayPal/PayPalStd.pm | 63 ++++++++++++++++--- .../i18n/English/PayDriver_PayPalStd.pm | 15 +++++ 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/lib/WebGUI/Shop/PayDriver/PayPal/PayPalStd.pm b/lib/WebGUI/Shop/PayDriver/PayPal/PayPalStd.pm index bc682a341..2043ca98b 100644 --- a/lib/WebGUI/Shop/PayDriver/PayPal/PayPalStd.pm +++ b/lib/WebGUI/Shop/PayDriver/PayPal/PayPalStd.pm @@ -3,7 +3,7 @@ package WebGUI::Shop::PayDriver::PayPal::PayPalStd; =head1 LEGAL ------------------------------------------------------------------- PayPal Standard payment driver for WebGUI. - Copyright (C) 2009 Invicta Services, LLC. + Copyright (C) 2009 Plain Black Corporation. ------------------------------------------------------------------- This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License @@ -28,6 +28,11 @@ use warnings; use base qw/WebGUI::Shop::PayDriver::PayPal/; use URI; +use URI::Escape; +use LWP::UserAgent; +use Readonly; + +Readonly my $I18N => 'PayDriver_PayPalStd'; =head1 NAME @@ -38,6 +43,11 @@ PayPal Website payments standard A PayPal Website payments standard handler for WebGUI. Provides an interface to PayPal with cart contents and transaction information on return. +=head2 IMPORTANT NOTE + +In order to use this module, Auto Return and PDT must be enabled in your +paypal seller account. If they are not, everything will break! + =head1 SYNOPSIS Add "WebGUI::Shop::PayDriver::PayPal::PayPalStd" to the paymentDrivers list in your WebGUI site config file. @@ -71,7 +81,7 @@ sub definition { unless ref $session eq 'WebGUI::Session'; my $definition = shift; - my $i18n = WebGUI::International->new( $session, 'PayDriver_PayPalStd' ); + my $i18n = WebGUI::International->new( $session, $I18N ); tie my %fields, 'Tie::IxHash'; %fields = ( @@ -85,6 +95,11 @@ sub definition { label => $i18n->get('signature'), hoverHelp => $i18n->get('signature help'), }, + identityToken => { + fieldType => 'text', + label => $i18n->get('identity token'), + hoverHelp => $i18n->get('identity token help'), + }, currency => { fieldType => 'selectBox', label => $i18n->get('currency'), @@ -212,13 +227,16 @@ sub paymentVariables { currency_code => $self->get('currency'), no_shipping => 1, - rm => 2, return => $return->as_string, cancel_return => $cancel->as_string, shipping => $cart->calculateShipping, tax_cart => $cart->calculateTaxes, discount_amount_cart => -($cart->calculateShopCreditDeduction), + + # When we verify that we have a valid transaction ID later on in + # processPayment, we'll make sure it's the cart we think it is. + custom => $cart->getId, ); my $counter = 0; @@ -260,20 +278,45 @@ passed to us. sub processPayment { my ( $self, $transaction ) = @_; my $session = $self->session; - my $params = $session->form->paramsHashRef; - my $status = $params->{payment_status}; - my $tx = $params->{txn_id}; - if ($status ne 'Completed') { + # To prevent a spoofed post to this url, we'll get the info from paypal + # instead of relying on what was passed to us. + my $tx = $session->form->process('tx'); + + my %form = ( + cmd => '_notify-synch', + tx => $tx, + at => $self->get('identityToken'), + ); + my $response = LWP::UserAgent->new->post($self->payPalUrl, \%form); + my ($status, @lines) = split("\n", uri_unescape($response->content)); + my %params = map { split /=/ } @lines; + + if ($status =~ /FAIL/) { my $message = ''; - foreach my $key ( keys %$params ) { - $message .= ""; + foreach my $key ( keys %params ) { + $message .= ""; } $message .= '
FieldValue
$key$params->{$key}
$key$params{$key}
'; return ( 0, $tx, $status, $message ); } - return ( 1, $tx, $status, $status ); + # Make sure the transaction is for this cart to prevent spoofing + my $cartId = $self->getCart->getId; + if ($params{custom} ne $cartId) { + my $user = $session->user; + my $name = $user->username; + my $id = $user->userId; + $session->log->warn("SECURITY WARNING: $name (id: $id) tried to " . + "checkout cart $cartId with PayPal transaction $tx, which " . + "did not match the cart we passed ($params{custom})"); + + my $i18n = WebGUI::International->new( $session, $I18N ); + return ( 0, $tx, 'FAIL', $i18n->get('cart transaction mismatch') ); + } + + $status = $params{payment_status}; + return ( 1, $tx, $status, $status, $status ); } ## end sub processPayment #------------------------------------------------------------------- diff --git a/lib/WebGUI/i18n/English/PayDriver_PayPalStd.pm b/lib/WebGUI/i18n/English/PayDriver_PayPalStd.pm index 433ec2885..df1bdcab0 100644 --- a/lib/WebGUI/i18n/English/PayDriver_PayPalStd.pm +++ b/lib/WebGUI/i18n/English/PayDriver_PayPalStd.pm @@ -24,6 +24,11 @@ package WebGUI::i18n::English::PayDriver_PayPalStd; use strict; our $I18N = { + 'cart transaction mismatch' => { + message => 'Cart mismatch detected. This incident will be logged.', + context => 'Message displayed when a transaction spoof is detected.', + lastUpdated => 1248289540, + }, 'error occurred message' => { message => q|The following errors occurred:|, lastUpdated => 0, @@ -40,6 +45,16 @@ our $I18N = { context => q|Default PayPal payment gateway label| }, + 'identity token' => { + message => 'PDT Identity Token', + lastUpdated => 1248297326, + }, + + 'identity token help' => { + message => q{The identity token listed under the Payment Data Transfer radio button in your website payment preference on PayPal}, + lastUpdated => 1248297326, + }, + 'vendorId' => { message => q|PayPal Account|, lastUpdated => 0,